|
From: Andrew B. <an...@ca...> - 2005-07-12 07:10:29
|
This patch adds selectOne and selectOneBy methods to SQLObject, and simple
test cases for them.
ForeignKey columns and alternateID columns are great, but don't satisfy all
use-cases. There are times when you need to use select or selectBy, but you
also know that your query can only ever return one (or zero) rows. An
example is any time you have an UNIQUE constraint on multiple columns, and
you do a selectBy on the same set of columns.
selectOne and selectOneBy are designed to make this situation as easy to
deal with as possible.
At Canonical, we've played with this for some time and found it to be a very
valuable addition to SQLObject, eliminating a lot of boilerplate code.
Please apply, or let me know what you think!
Index: SQLObject/sqlobject/main.py
===================================================================
--- SQLObject.orig/sqlobject/main.py 2005-06-30 12:26:44.000000000 -0300
+++ SQLObject/sqlobject/main.py 2005-06-30 12:26:46.000000000 -0300
@@ -49,6 +49,7 @@
class SQLObjectNotFound(LookupError): pass
class SQLObjectIntegrityError(Exception): pass
+class SQLObjectMoreThanOneResultError(Exception): pass
True, False = 1==1, 0==1
@@ -1186,6 +1187,46 @@
selectBy = classmethod(selectBy)
+ def selectOne(cls, clause=None, clauseTables=None, lazyColumns=False,
+ connection=None):
+ """A variant of select to return a single result.
+
+ If clause finds no results, this returns None. If it finds one result,
+ it returns it. If it finds more than one result, it raises a
+ SQLObjectMoreThanOneResultError.
+ """
+ results = list(SelectResults(cls, clause, clauseTables=clauseTables,
+ lazyColumns=lazyColumns,
+ connection=connection))
+ if len(results) == 0:
+ return None
+ elif len(results) == 1:
+ return results[0]
+ else:
+ raise SQLObjectMoreThanOneResultError(
+ "%d rows retrieved by selectOne" % len(results))
+ selectOne = classmethod(selectOne)
+
+ def selectOneBy(cls, lazyColumns=False, connection=None, **kw):
+ """A variant of selectBy to return a single result.
+
+ If it finds no results, this returns None. If it finds one result,
+ it returns it. If it finds more than one result, it raises a
+ SQLObjectMoreThanOneResultError.
+ """
+ results = list(SelectResults(cls,
+ cls._connection._SO_columnClause(cls, kw),
+ lazyColumns=lazyColumns,
+ connection=connection))
+ if len(results) == 0:
+ return None
+ elif len(results) == 1:
+ return results[0]
+ else:
+ raise SQLObjectMoreThanOneResultError(
+ "%d rows retrieved by selectOne" % len(results))
+ selectOneBy = classmethod(selectOneBy)
+
def dropTable(cls, ifExists=False, dropJoinTables=True, cascade=False,
connection=None):
conn = connection or cls._connection
@@ -1433,4 +1474,5 @@
__all__ = ['NoDefault', 'SQLObject', 'sqlmeta',
'getID', 'getObject',
- 'SQLObjectNotFound', 'sqlhub']
+ 'SQLObjectNotFound', 'SQLObjectMoreThanOneResultError',
+ 'sqlhub']
Index: SQLObject/sqlobject/tests/test_select.py
===================================================================
--- SQLObject.orig/sqlobject/tests/test_select.py 2005-06-30 12:26:44.000000000 -0300
+++ SQLObject/sqlobject/tests/test_select.py 2005-06-30 12:27:18.000000000 -0300
@@ -1,3 +1,5 @@
+from py.test import raises
+
from sqlobject import *
from sqlobject.tests.dbtest import *
@@ -112,3 +114,17 @@
# Ensure that the orderBy argument to selectBy works
assert ([c.n2 for c in Counter2.selectBy(n1=5, orderBy='-n2')] ==
range(10)[::-1])
+
+ def testSelectOne(self):
+ # Test the behaviour of selectOne when it gets 0 rows, 1 row, and >1
+ # rows.
+ assert Counter2.selectOne('n1=99') == None
+ raises(SQLObjectMoreThanOneResultError, Counter2.selectOne, 'n1=1')
+ assert Counter2.selectOne('n1=1 AND n2=2').n1 == 1
+
+ # Test the behaviour of selectOneBy when it gets 0 rows, 1 row, and >1
+ # rows.
+ assert Counter2.selectOneBy(n1=99) == None
+ raises(SQLObjectMoreThanOneResultError, Counter2.selectOneBy, n1=1)
+ assert Counter2.selectOneBy(n1=1, n2=2).n1 == 1
+
-Andrew.
|
|
From: Oleg B. <ph...@ph...> - 2005-07-14 09:00:57
|
On Tue, Jul 12, 2005 at 05:10:23PM +1000, Andrew Bennetts wrote:
> This patch adds selectOne and selectOneBy methods to SQLObject, and simple
> test cases for them.
> + if len(results) == 0:
> + return None
> + elif len(results) == 1:
> + return results[0]
> + else:
> + raise SQLObjectMoreThanOneResultError(
> + "%d rows retrieved by selectOne" % len(results))
You have named it selectOne, but allows empty reu;st. It is
selectZeroOrOne, actually.
But in any case I don't like the idea. It does not add much. You'd
better implement it in a base class in your application.
Oleg.
--
Oleg Broytmann http://phd.pp.ru/ ph...@ph...
Programmers don't die, they just GOSUB without RETURN.
|
|
From: Andrew B. <an...@ca...> - 2005-07-14 11:26:25
|
On Thu, Jul 14, 2005 at 01:00:52PM +0400, Oleg Broytmann wrote:
> On Tue, Jul 12, 2005 at 05:10:23PM +1000, Andrew Bennetts wrote:
> > This patch adds selectOne and selectOneBy methods to SQLObject, and simple
> > test cases for them.
>
> > + if len(results) == 0:
> > + return None
> > + elif len(results) == 1:
> > + return results[0]
> > + else:
> > + raise SQLObjectMoreThanOneResultError(
> > + "%d rows retrieved by selectOne" % len(results))
>
> You have named it selectOne, but allows empty reu;st. It is
> selectZeroOrOne, actually.
Yeah, it can explicitly return None. Perhaps it would be better have a
default= keyword argument, and return that, or raise SQLObjectNotFound if a
default isn't given. Think of it as analagous to dict.get. And
selectZeroOrOne is a much more unwieldy name.
For most of our uses of it, the current behaviour seems to be ideal.
> But in any case I don't like the idea. It does not add much. You'd
> better implement it in a base class in your application.
It's used over 100 times in our code (for Launchpad). That would be a lot
tedious code without selectOne/selectOneBy!
We can keep it as a specific extension in our code, but I think this useful
for more than just us. Has no-one else ever found themselves writing code
like this:
results = Foo.select(...)
try:
# XXX: we really ought to assert there's no more than one foo...
the_foo = results[0]
except IndexError:
raise FooNotFound
return the_foo
several times?
In our code, this happens all the time, in places where alternateIDs and
SQLObject.get and so on don't help.
In fact, this can also be used instead of alternateIDs and get, which might
using SQLObject simpler and easier to learn... I'm not sure if this is a
good idea or not, but it is tempting.
-Andrew.
|
|
From: Oleg B. <ph...@ph...> - 2005-07-14 12:13:59
|
On Thu, Jul 14, 2005 at 09:26:53PM +1000, Andrew Bennetts wrote:
> >
> > > + if len(results) == 0:
> > > + return None
> > > + elif len(results) == 1:
> > > + return results[0]
> > > + else:
> > > + raise SQLObjectMoreThanOneResultError(
> > > + "%d rows retrieved by selectOne" % len(results))
>
> We can keep it as a specific extension in our code, but I think this useful
> for more than just us.
Well, what people think? Is selectOne() really valuable addition?
Oleg.
--
Oleg Broytmann http://phd.pp.ru/ ph...@ph...
Programmers don't die, they just GOSUB without RETURN.
|
|
From: Sidnei da S. <si...@en...> - 2005-07-14 12:19:50
|
On Thu, Jul 14, 2005 at 04:13:51PM +0400, Oleg Broytmann wrote: | > We can keep it as a specific extension in our code, but I think this useful | > for more than just us. | | Well, what people think? Is selectOne() really valuable addition? Yes, I think it's very valuable. -- Sidnei da Silva Enfold Systems, LLC. http://enfoldsystems.com |
|
From: Kevin D. <da...@gm...> - 2005-07-14 13:12:30
|
On 7/14/05, Oleg Broytmann <ph...@ph...> wrote: > On Thu, Jul 14, 2005 at 09:26:53PM +1000, Andrew Bennetts wrote: > > > > > > > + if len(results) =3D=3D 0: > > > > + return None > > > > + elif len(results) =3D=3D 1: > > > > + return results[0] > > > > + else: > > > > + raise SQLObjectMoreThanOneResultError( > > > > + "%d rows retrieved by selectOne" % len(results)) > > > > We can keep it as a specific extension in our code, but I think this us= eful > > for more than just us. >=20 > Well, what people think? Is selectOne() really valuable addition? Yes, I think it's a nice convenience. For me, most uses of this are taken care of by the alternateID byFoo methods, but there are still times when this would be handy. Kevin |
|
From: <da...@eg...> - 2005-07-14 21:50:01
|
I also think that the functionality is common enough that explicit support for it in SQLObject is useful. However, I'm not sure that the actual implementation suggested is the ideal interface into this functionality. One of the things that appealed to me about SQLObject to begin was that it felt "pythonic". I think TOOWTDI and EIBTI are important guidelines to follow. I had in fact coded basically this same patch and also the selectBy(orderBy=) patch posted by Andrew, but then backed out of it based on the above guidelines. The argument for backing out of the selectBy(orderBy=) is I think the more compelling one. Since selectBy is by design intended to allow use of keyword args to specify equality constraints, I think adding additional kwargs which are "special" adds too much ambiguity. In fact, I would go so far as to say I think selectBy should be deprecated in favor of extending select() to take *args instead of the current single clause= and doing an implicit AND of the vararg clauses. I realize that (AFAICT) there are some semantic differences in how select() and selectBy() work; namely, selectBy() deals with foreign key constraints transparently while select() requires you to condition on ..ID. However, I think this is something that should be fixed in select(), rather than adding a proliferation of methods which only add minor ease-of-use tweaks. my 2c, d On Jul 14, 2005, at 6:12 AM, Kevin Dangoor wrote: > On 7/14/05, Oleg Broytmann <ph...@ph...> wrote: >> On Thu, Jul 14, 2005 at 09:26:53PM +1000, Andrew Bennetts wrote: >>>> >>>>> + if len(results) == 0: >>>>> + return None >>>>> + elif len(results) == 1: >>>>> + return results[0] >>>>> + else: >>>>> + raise SQLObjectMoreThanOneResultError( >>>>> + "%d rows retrieved by selectOne" % len(results)) >>> >>> We can keep it as a specific extension in our code, but I think this >>> useful >>> for more than just us. >> >> Well, what people think? Is selectOne() really valuable addition? > > Yes, I think it's a nice convenience. For me, most uses of this are > taken care of by the alternateID byFoo methods, but there are still > times when this would be handy. > > Kevin > > > ------------------------------------------------------- > This SF.Net email is sponsored by the 'Do More With Dual!' webinar > happening > July 14 at 8am PDT/11am EDT. We invite you to explore the latest in > dual > core and dual graphics technology at this free one hour event hosted > by HP, > AMD, and NVIDIA. To register visit http://www.hp.com/go/dualwebinar > _______________________________________________ > sqlobject-discuss mailing list > sql...@li... > https://lists.sourceforge.net/lists/listinfo/sqlobject-discuss > |
|
From: Oleg B. <ph...@ph...> - 2005-07-14 22:34:26
|
On Thu, Jul 14, 2005 at 02:50:04PM -0700, da...@eg... wrote:
> In fact, I would go so far as to say I think selectBy
> should be deprecated in favor of extending select()
> to take *args instead of the current single clause=
> and doing an implicit AND of the vararg clauses.
Ahem... You are talking about selectBy() with the subject "selectOne". I
am totally confused about what you are for and what you are against!
Oleg.
--
Oleg Broytmann http://phd.pp.ru/ ph...@ph...
Programmers don't die, they just GOSUB without RETURN.
|
|
From: <da...@eg...> - 2005-07-14 23:27:30
|
On Jul 14, 2005, at 3:31 PM, Oleg Broytmann wrote: > > Ahem... You are talking about selectBy() with the subject > "selectOne". I > am totally confused about what you are for and what you are against! Sorry if I wasn't clear. I apologize that I merged my response to two threads into a response to just one. What I meant was: Andrew Bennetts proposed some patches which includes functionality that I've wanted in the past (e.g., selectOne()). I am *for* incorporating that functionality into SQLObject. I am *against* the way in which the patches expose the functionality to the user (i.e., the API). In particular, I am *against* adding lots of selectFOO methods, and am instead *for* extending select() to incorporate the functionality. That's my basic point. In addition, I did try to make the argument that I thought selectBy() was an ease-of-use API that might lead to more problems than it solves. I have, like Andrew, wanted to give selectBy() some of the kwargs that select() takes. But since selectBy() uses kwargs for its clause specification, this causes inherent ambiguity in what the user means. I was trying to suggest that perhaps the better approach was to allow easier use of select() instead of adding functionality to selectBy(). I hope that's clearer. For the selectOne() functionality I would suggest adding the kwarg 'unique' to select(). d |
|
From: Andrew B. <an...@ca...> - 2005-07-15 00:54:03
|
On Thu, Jul 14, 2005 at 04:28:19PM -0700, da...@eg... wrote:
[...]
> That's my basic point. In addition, I did try to make
> the argument that I thought selectBy() was an ease-of-use
> API that might lead to more problems than it solves.
> I have, like Andrew, wanted to give selectBy() some
> of the kwargs that select() takes. But since selectBy()
> uses kwargs for its clause specification, this causes
> inherent ambiguity in what the user means. I was
> trying to suggest that perhaps the better approach
> was to allow easier use of select() instead of adding
> functionality to selectBy().
I don't think this is a serious problem with selectBy. It's unlikely that
column names will clash with those keyword arguments. If it is a problem,
we could use a convention like treating "orderBy_" as meaning a column named
"orderBy" -- I think I've seen that used in e.g. Nevow's "stan" API, to
workaround the problem of "class" being a reserved word in Python but also a
common attribute in HTML.
> I hope that's clearer. For the selectOne() functionality
> I would suggest adding the kwarg 'unique' to select().
I don't like this -- magic flags that make a method return a different type
of result smell bad to me. The return type of a method ought to be
consistent.
Another way to do this would be a method of SelectResults, but that feels
like an odd place for it:
foo = Foo.select(query).getOneOrNone()
I share your concerns about "there should be only one way to do it", but in
this case I think practicality beats purity.
-Andrew.
|
|
From: <da...@eg...> - 2005-07-15 01:39:58
|
On Jul 14, 2005, at 5:54 PM, Andrew Bennetts wrote:
> On Thu, Jul 14, 2005 at 04:28:19PM -0700, da...@eg... wrote:
> I don't think this is a serious problem with selectBy. It's unlikely
> that
> column names will clash with those keyword arguments. If it is a
> problem,
> we could use a convention like treating "orderBy_" as meaning a column
> named
> "orderBy" -- I think I've seen that used in e.g. Nevow's "stan" API, to
> workaround the problem of "class" being a reserved word in Python but
> also a
> common attribute in HTML.
For me the question of column names clashing
with the keywords is less of the issue than the
simple fact of using kwargs for two different
purposes. I also implemented what you have,
but ended up looking at code like:
r = t.selectBy(col=val,
orderBy=ordercol,
this=that,
connection=c,
)
Now obviously I would regroup and make sure
that the "clause" kwargs came before all the
"option" kwargs, but after using this a bit I
decided the convenience just wasn't worth
the "ugly!" chorus in the back of my mind
and moved all of my use cases to select()
and reverted my mods.
I appreciate this is a matter of taste and
that I don't personally have to use selectBy()
in that way even if the extension patch gets in,
but that's my take on it.
>> I hope that's clearer. For the selectOne() functionality
>> I would suggest adding the kwarg 'unique' to select().
> I don't like this -- magic flags that make a method return a different
> type
> of result smell bad to me. The return type of a method ought to be
> consistent.
You're right on this one - I hadn't thought it
through enough. I retract my suggestion,
but would still prefer not proliferating selectXXX
methods.
> Another way to do this would be a method of SelectResults, but that
> feels
> like an odd place for it:
>
> foo = Foo.select(query).getOneOrNone()
Yeah, I'm not so sure about this either.
> I share your concerns about "there should be only one way to do it",
> but in
> this case I think practicality beats purity.
I think you may be right for the selectOne() problem
(although I do wish someone would come up with
an elegant solution!).
> -Andrew.
>
|
|
From: Andrew B. <an...@ca...> - 2005-07-15 03:36:50
|
On Thu, Jul 14, 2005 at 06:40:52PM -0700, da...@eg... wrote:
> On Jul 14, 2005, at 5:54 PM, Andrew Bennetts wrote:
> >On Thu, Jul 14, 2005 at 04:28:19PM -0700, da...@eg... wrote:
> >I don't think this is a serious problem with selectBy. It's unlikely
> >that column names will clash with those keyword arguments. If it is a
> >problem, we could use a convention like treating "orderBy_" as meaning a
> >column named "orderBy" -- I think I've seen that used in e.g. Nevow's
> >"stan" API, to workaround the problem of "class" being a reserved word in
> >Python but also a common attribute in HTML.
>
> For me the question of column names clashing
> with the keywords is less of the issue than the
> simple fact of using kwargs for two different
> purposes. I also implemented what you have,
> but ended up looking at code like:
>
> r = t.selectBy(col=val,
> orderBy=ordercol,
> this=that,
> connection=c,
> )
[...]
Oh, I see. I understand your point now. Thanks for the clarification!
I guess selectBy could be superseded by select with a "byCols" helper, that
would look like:
r = t.select(byCols(col=val), orderBy=ordercol, etc=etc)
But it feels a bit ugly to me. I guess the byCols helper would be part of
SQLBuilder.
[...]
> >I share your concerns about "there should be only one way to do it", but
> >in this case I think practicality beats purity.
>
> I think you may be right for the selectOne() problem
> (although I do wish someone would come up with
> an elegant solution!).
Me too! I'd love to see an idea that makes everyone instantly happy here,
but until then I'll keep advocating for selectOne.
Thanks for your thoughts!
-Andrew.
|
|
From: Oleg B. <ph...@ma...> - 2005-07-15 13:48:29
|
On Fri, Jul 15, 2005 at 01:37:23PM +1000, Andrew Bennetts wrote:
> r = t.select(byCols(col=val), orderBy=ordercol, etc=etc)
.selectBy() returns a SelectResults that can be transformed:
r = t.selectBy(col=val).orderBy(ordercol)
Oleg.
--
Oleg Broytmann http://phd.pp.ru/ ph...@ph...
Programmers don't die, they just GOSUB without RETURN.
|
|
From: Evandro V. M. <ev...@as...> - 2005-07-15 14:48:00
|
Can anyone tell me if I can check if a database exists using SQLObject routines ? I'm using Postgres and there is an easy way to check that through psycopg, which is not a good idea since I want to support many others databases in my system. Is there a general approach for that ? -- Evandro Vale Miquelito : ev...@as... Async Open Source - Brazil | http://www.async.com.br |
|
From: Oleg B. <ph...@ma...> - 2005-07-15 16:01:40
|
On Fri, Jul 15, 2005 at 11:45:42AM -0300, Evandro Vale Miquelito wrote:
> Can anyone tell me if I can check if a database exists using SQLObject
> routines ? I'm using Postgres and there is an easy way to check that
> through psycopg, which is not a good idea since I want to support many
> others databases in my system. Is there a general approach for that ?
There is no. To use SQLObject you need a connection... but where are you
going to connect to if there is no a database?
Oleg.
--
Oleg Broytmann http://phd.pp.ru/ ph...@ph...
Programmers don't die, they just GOSUB without RETURN.
|
|
From: Evandro V. M. <ev...@as...> - 2005-07-15 17:40:43
|
Oleg Broytmann wrote: > On Fri, Jul 15, 2005 at 11:45:42AM -0300, Evandro Vale Miquelito wrote: > >>Can anyone tell me if I can check if a database exists using SQLObject >>routines ? I'm using Postgres and there is an easy way to check that >>through psycopg, which is not a good idea since I want to support many >>others databases in my system. Is there a general approach for that ? > > > There is no. To use SQLObject you need a connection... but where are you > going to connect to if there is no a database? I just would like to catch this kind of exception and show to user a proper message. The system I'm currently writing has postgres as a dependency but we can't ensure that users has been setup the database before start the system and that's what I want to inform. As I can see, the only way I can do that is checking the database system(postgres, mysql...) and verifying specific exceptions. Evandro Vale Miquelito : ev...@as... Async Open Source - Brazil | http://www.async.com.br |
|
From: Oleg B. <ph...@ma...> - 2005-07-15 18:04:06
|
On Fri, Jul 15, 2005 at 02:39:15PM -0300, Evandro Vale Miquelito wrote:
> As I can see, the only way I can do that is checking the database
> system(postgres, mysql...) and verifying specific exceptions.
In Postgres you can do the ususal trick - connect to template1 database
and list availabale databases.
In all databases including Postgres distinguish connection error (no
server on the host:port), no db error and auth error.
Oleg.
--
Oleg Broytmann http://phd.pp.ru/ ph...@ph...
Programmers don't die, they just GOSUB without RETURN.
|
|
From: Jamie W. <ja...@sp...> - 2005-07-15 01:27:40
|
This one time, at band camp, Oleg Broytmann wrote: >On Thu, Jul 14, 2005 at 09:26:53PM +1000, Andrew Bennetts wrote: >> > >> > > + if len(results) == 0: >> > > + return None >> > > + elif len(results) == 1: >> > > + return results[0] >> > > + else: >> > > + raise SQLObjectMoreThanOneResultError( >> > > + "%d rows retrieved by selectOne" % len(results)) >> >> We can keep it as a specific extension in our code, but I think this useful >> for more than just us. > > Well, what people think? Is selectOne() really valuable addition? I like it; I replicate this sort of code all over the place too. |
|
From: Sidnei da S. <si...@en...> - 2005-07-14 12:19:44
|
On Thu, Jul 14, 2005 at 09:26:53PM +1000, Andrew Bennetts wrote: | We can keep it as a specific extension in our code, but I think this useful | for more than just us. Has no-one else ever found themselves writing code | like this: | | results = Foo.select(...) | try: | # XXX: we really ought to assert there's no more than one foo... | the_foo = results[0] | except IndexError: | raise FooNotFound | return the_foo | | several times? /me raises several hands -- Sidnei da Silva Enfold Systems, LLC. http://enfoldsystems.com |
|
From: Tom C. <su...@ic...> - 2005-07-14 12:48:02
|
On Thursday 14 July 2005 14:17, Sidnei da Silva wrote: > On Thu, Jul 14, 2005 at 09:26:53PM +1000, Andrew Bennetts wrote: > | We can keep it as a specific extension in our code, but I think this > | useful for more than just us. Has no-one else ever found themselves > | writing code like this: > | > | results = Foo.select(...) > | try: > | # XXX: we really ought to assert there's no more than one foo... > | the_foo = results[0] > | except IndexError: > | raise FooNotFound > | return the_foo > | > | several times? > > /me raises several hands Same here.... |
|
From: Oleg B. <ph...@ph...> - 2005-07-18 07:34:11
|
On Tue, Jul 12, 2005 at 05:10:23PM +1000, Andrew Bennetts wrote:
> + def selectOne(cls, clause=None, clauseTables=None, lazyColumns=False,
> + connection=None):
> + results = list(SelectResults(cls, clause, clauseTables=clauseTables,
> + lazyColumns=lazyColumns,
> + connection=connection))
> + selectOne = classmethod(selectOne)
Please add "distinct" and "join" keywords. Look at .select() method.
Oleg.
--
Oleg Broytmann http://phd.pp.ru/ ph...@ph...
Programmers don't die, they just GOSUB without RETURN.
|
