python-sql: select examples

[nicoe, 2011-07-24 10:07:41]

http://codereview.appspot.com/4248045/diff/15001/examples.rst
File examples.rst (right):

http://codereview.appspot.com/4248045/diff/15001/examples.rst#newcode28
examples.rst:28: >>> select.where = user.name == 'foo'
I prefer select.where(*conditions) because it does not have the effect of
polluting select with conditions.

http://codereview.appspot.com/4248045/diff/15001/examples.rst#newcode38
examples.rst:38: >>> join = Join('user_group')
Maybe it would be intersting to be able to also add conditions when creating a
join.

http://codereview.appspot.com/4248045/diff/15001/examples.rst#newcode40
examples.rst:40: >>> select = (user + join).select(user.name, join.group)
We should add test for the case where condition on join is not defined.
It should probably raise an error when doing a '+'.

[ced, 2011-07-24 10:17:33]

http://codereview.appspot.com/4248045/diff/15001/examples.rst
File examples.rst (right):

http://codereview.appspot.com/4248045/diff/15001/examples.rst#newcode28
examples.rst:28: >>> select.where = user.name == 'foo'
On 2011/07/24 10:07:41, nicoe wrote:
> I prefer select.where(*conditions) because it does not have the effect of
> polluting select with conditions.

I don't understand. Any way, select will need to handle the where clause.

http://codereview.appspot.com/4248045/diff/15001/examples.rst#newcode38
examples.rst:38: >>> join = Join('user_group')
On 2011/07/24 10:07:41, nicoe wrote:
> Maybe it would be intersting to be able to also add conditions when creating a
> join.

Not possible because you need to have the Join table to express the condition

http://codereview.appspot.com/4248045/diff/15001/examples.rst#newcode40
examples.rst:40: >>> select = (user + join).select(user.name, join.group)
On 2011/07/24 10:07:41, nicoe wrote:
> We should add test for the case where condition on join is not defined.
> It should probably raise an error when doing a '+'.

It is allow to make join without condition

[nicoe, 2011-07-24 13:38:42]

http://codereview.appspot.com/4248045/diff/15001/examples.rst
File examples.rst (right):

http://codereview.appspot.com/4248045/diff/15001/examples.rst#newcode28
examples.rst:28: >>> select.where = user.name == 'foo'
On 2011/07/24 10:17:33, ced wrote:
> On 2011/07/24 10:07:41, nicoe wrote:
> > I prefer select.where(*conditions) because it does not have the effect of
> > polluting select with conditions.
> 
> I don't understand. Any way, select will need to handle the where clause.

Imagine this kind of code:

s = user.select()
for condition in conditions:
    ...
    s.where = condition
    ...

at the end of the loop s.where will either have a value or not according to the
content of conditions. I would prefer that where is a method to prevent this.

http://codereview.appspot.com/4248045/diff/15001/examples.rst#newcode40
examples.rst:40: >>> select = (user + join).select(user.name, join.group)
On 2011/07/24 10:17:33, ced wrote:
> On 2011/07/24 10:07:41, nicoe wrote:
> > We should add test for the case where condition on join is not defined.
> > It should probably raise an error when doing a '+'.
> 
> It is allow to make join without condition

Indeed

[ced, 2011-07-24 13:46:01]

http://codereview.appspot.com/4248045/diff/15001/examples.rst
File examples.rst (right):

http://codereview.appspot.com/4248045/diff/15001/examples.rst#newcode28
examples.rst:28: >>> select.where = user.name == 'foo'
On 2011/07/24 13:38:43, nicoe wrote:
> On 2011/07/24 10:17:33, ced wrote:
> > On 2011/07/24 10:07:41, nicoe wrote:
> > > I prefer select.where(*conditions) because it does not have the effect of
> > > polluting select with conditions.
> > 
> > I don't understand. Any way, select will need to handle the where clause.
> 
> Imagine this kind of code:
> 
> s = user.select()
> for condition in conditions:
>     ...
>     s.where = condition
>     ...
> 
> at the end of the loop s.where will either have a value or not according to
the
> content of conditions. I would prefer that where is a method to prevent this.

I don't see why a method will be better in any way where will be set.

[thomas.sharoon, 2011-07-28 00:25:55]

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py
File pysql/__init__.py (right):

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode67
pysql/__init__.py:67: 
one more <BLANKLINE> to ensure consistency of space b/w classes

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode89
pysql/__init__.py:89: for_=None):
each parameter seems to have more than obvious constraints in being used. Doc
strings explaining the use and how to use each param would be useful ?

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode90
pysql/__init__.py:90: FromItem.__init__(self)
why not use super ? :class:Query doesn't seem to do anything special to prevent
from using regular super()

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode91
pysql/__init__.py:91: assert all(isinstance(col, Column) for col in columns)
An error message could be provided like "All columns must be instances of
Column"

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode95
pysql/__init__.py:95: assert isinstance(where, (Condition, And, Or))
Assert could return message ?

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode106
pysql/__init__.py:106: self.order_by = order_by
Not validating if oder_by is instance(Order) ?

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode133
pysql/__init__.py:133: if self.order_by:
order by seems to be broken

>>> q = Select([user.name], user, order_by=Asc(user.name))
>>> q
<pysql.Select object at 0x10a2f6f10>
>>> str(q)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pysql/__init__.py", line 134, in __str__
    order_by = ' ORDER BY ' + ', '.join(map(str, self.order_by))
TypeError: argument 2 to map() must support iteration
>>> q.order_by
<pysql.Asc object at 0x10a2fe250>
>>> q = Select([user.name], user, order_by=user.name)
>>> q.order_by
<pysql.Column object at 0x10a2f6750>
>>> str(q)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pysql/__init__.py", line 134, in __str__
    order_by = ' ORDER BY ' + ', '.join(map(str, self.order_by))
TypeError: argument 2 to map() must support iteration

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode204
pysql/__init__.py:204: self.only = bool(only)
why is it cast into boolean again ? won't the values be True or False ?

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode214
pysql/__init__.py:214: only = ''
can be simplified to 

only = ' ONLY ' if self.only else ''

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode260
pysql/__init__.py:260: def convert(condition):
what does this function do ?

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode343
pysql/__init__.py:343: def __eq__(self, other):
loved the idea!

[thomas.sharoon, 2011-07-28 00:30:09]

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py
File pysql/__init__.py (right):

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode106
pysql/__init__.py:106: self.order_by = order_by
if order_by is not None:
     assert all(isinstance(arg, Order) for arg in order_by)
self.order_by = order_by

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode133
pysql/__init__.py:133: if self.order_by:
Well I figured that out

>>> q = Select([user.name], user, order_by=[Asc(user.name)])
>>> str(q)
'SELECT "a"."name" FROM "user" AS "a" ORDER BY "a"."name" ASC'

So the problem is that the value of self.order_by is not sanitized by __init__

[nicoe, 2011-07-28 08:33:45]

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py
File pysql/__init__.py (right):

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode411
pysql/__init__.py:411: 'LIKE', 'NOT LIKE', 'ILIKE', 'NOT ILIKE', 'IS NULL', 'IS
NOT NULL')
What about the BETWEEN operator ?

http://codereview.appspot.com/4248045/diff/24002/pysql/aggregate.py
File pysql/aggregate.py (right):

http://codereview.appspot.com/4248045/diff/24002/pysql/aggregate.py#newcode11
pysql/aggregate.py:11: _sql = ''
_sql might be a property for multi backend support

http://codereview.appspot.com/4248045/diff/24002/pysql/aggregate.py#newcode24
pysql/aggregate.py:24: class Bit_and(Aggregate):
I prefer BitAnd, BitOr, BoolAnd, ... looks more pythonic

[ced, 2011-07-28 21:47:41]

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py
File pysql/__init__.py (right):

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode67
pysql/__init__.py:67: 
On 2011/07/28 00:25:55, thomas.sharoon wrote:
> one more <BLANKLINE> to ensure consistency of space b/w classes

Done.

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode90
pysql/__init__.py:90: FromItem.__init__(self)
On 2011/07/28 00:25:55, thomas.sharoon wrote:
> why not use super ? :class:Query doesn't seem to do anything special to
prevent
> from using regular super()

Done.

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode91
pysql/__init__.py:91: assert all(isinstance(col, Column) for col in columns)
On 2011/07/28 00:25:55, thomas.sharoon wrote:
> An error message could be provided like "All columns must be instances of
> Column"

The assert test seems obvious.

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode95
pysql/__init__.py:95: assert isinstance(where, (Condition, And, Or))
On 2011/07/28 00:25:55, thomas.sharoon wrote:
> Assert could return message ?

See above

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode95
pysql/__init__.py:95: assert isinstance(where, (Condition, And, Or))
On 2011/07/28 00:25:55, thomas.sharoon wrote:
> Assert could return message ?

See above

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode106
pysql/__init__.py:106: self.order_by = order_by
On 2011/07/28 00:30:09, thomas.sharoon wrote:
> if order_by is not None:
>      assert all(isinstance(arg, Order) for arg in order_by)
> self.order_by = order_by

Done.

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode133
pysql/__init__.py:133: if self.order_by:
On 2011/07/28 00:30:09, thomas.sharoon wrote:
> Well I figured that out
> 
> >>> q = Select([user.name], user, order_by=[Asc(user.name)])
> >>> str(q)
> 'SELECT "a"."name" FROM "user" AS "a" ORDER BY "a"."name" ASC'
> 
> So the problem is that the value of self.order_by is not sanitized by __init__

Done.

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode204
pysql/__init__.py:204: self.only = bool(only)
On 2011/07/28 00:25:55, thomas.sharoon wrote:
> why is it cast into boolean again ? won't the values be True or False ?

Done.

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode214
pysql/__init__.py:214: only = ''
On 2011/07/28 00:25:55, thomas.sharoon wrote:
> can be simplified to 
> 
> only = ' ONLY ' if self.only else ''

Done.

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode260
pysql/__init__.py:260: def convert(condition):
On 2011/07/28 00:25:55, thomas.sharoon wrote:
> what does this function do ?

Done.

http://codereview.appspot.com/4248045/diff/24002/pysql/__init__.py#newcode411
pysql/__init__.py:411: 'LIKE', 'NOT LIKE', 'ILIKE', 'NOT ILIKE', 'IS NULL', 'IS
NOT NULL')
On 2011/07/28 08:33:48, nicoe wrote:
> What about the BETWEEN operator ?

Add TODO

http://codereview.appspot.com/4248045/diff/24002/pysql/aggregate.py
File pysql/aggregate.py (right):

http://codereview.appspot.com/4248045/diff/24002/pysql/aggregate.py#newcode11
pysql/aggregate.py:11: _sql = ''
On 2011/07/28 08:33:48, nicoe wrote:
> _sql might be a property for multi backend support

I don't think it is the way to go for multi-backend.
The equivalent function could be more complex than just an other method.

http://codereview.appspot.com/4248045/diff/24002/pysql/aggregate.py#newcode24
pysql/aggregate.py:24: class Bit_and(Aggregate):
On 2011/07/28 08:33:48, nicoe wrote:
> I prefer BitAnd, BitOr, BoolAnd, ... looks more pythonic

Done.

[thomas.sharoon, 2011-07-30 15:31:42]

http://codereview.appspot.com/4248045/diff/9006/examples.rst
File examples.rst (right):

http://codereview.appspot.com/4248045/diff/9006/examples.rst#newcode9
examples.rst:9: Select Statement
no example of order_by

http://codereview.appspot.com/4248045/diff/9006/pysql/__init__.py
File pysql/__init__.py (right):

http://codereview.appspot.com/4248045/diff/9006/pysql/__init__.py#newcode109
pysql/__init__.py:109: if order_by is not None:
how do you specify Asc/Desc ?