Issue: select .. where .. - recursive iterable parsing in where clause
Author: akaRem
State: closed
Created: 2013-11-16 21:50:09

Im on Mac OS X 10.9 with PyPy 2.1.0 and PostgreSQL 9.3.1, having
peewee 2.1.5 and psycopg2ct 2.4.4
I study how peewee builds SQL strings

Trying to implement postgresql's ltree:

```
class IntLTreeField(peewee.Field):
    db_field = 'ltree'
    template = '%(column_type)s'

    def field_attributes(self):
        return {
            'length': 6,
        }

    def db_value(self, value):
        formatter = "{{:0{length:}}}".format(**self.attributes)
        return '.'.join(map(formatter.format, value))

    def python_value(self, value):
        return map(int, value.split('.'))
```

Then - watch the SQL:

```
print MyModel.select().where(MyModel.some_tree == [(1,2),(3,4)]).sql()
```

Stacktrace:

```
/Users/user/Projects/pypy/bin/pypy /Users/user/Projects/pypy-flask/peewee-test.py
('SELECT t1."id", t1."some_value", t1."some_tree" FROM "mymodel" AS t1 WHERE (t1."some_value" = %s)', [6])
CREATE TABLE "mymodel" ("id" INTEGER NOT NULL PRIMARY KEY, "some_value" INTEGER NOT NULL, "some_tree" LTREE NOT NULL)
Traceback (most recent call last):
  File "app_main.py", line 72, in run_toplevel
  File "/Users/user/Projects/pypy-flask/peewee-test.py", line 60, in <module>
    print MyModel.select().where(MyModel.some_tree == [(1,2),(3,4)]).sql()
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 1686, in sql
1
    return self.compiler().generate_select(self)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 987, in generate_select
    where, w_params = self.parse_query_node(query._where, alias_map)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 905, in parse_query_node
    return self.parse_node(node, alias_map)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 863, in parse_node
    sql, params, unknown = self._parse(node, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/playhouse/postgres_ext.py", line 140, in _parse
    node, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 819, in _parse
    rhs, rparams = self.parse_node(node.rhs, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 863, in parse_node
    sql, params, unknown = self._parse(node, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/playhouse/postgres_ext.py", line 140, in _parse
    node, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 847, in _parse
    sql, params = self.parse_node_list(node, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 880, in parse_node_list
    node_sql, node_params = self.parse_node(node, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 863, in parse_node
    sql, params, unknown = self._parse(node, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/playhouse/postgres_ext.py", line 140, in _parse
    node, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 847, in _parse
    sql, params = self.parse_node_list(node, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 880, in parse_node_list
    node_sql, node_params = self.parse_node(node, alias_map, conv)
  File "/Users/user/Projects/pypy/site-packages/peewee.py", line 865, in parse_node
    params = [conv.db_value(i) for i in params]
  File "/Users/user/Projects/pypy-flask/peewee-test.py", line 32, in db_value
    return '.'.join(map(formatter.format, value))
TypeError: 'int' object is not iterable

Process finished with exit code 1
```

.. `MyModel.select().where(MyModel.some_tree == [1,2]).sql()` raises too; and even with `[((1,2),),((3,4),)]` or `[1,2]`

The problem looks like parser tries to recursive convert each item from each nested iterable in db_value(..). And it tries to make SQL like this: `SELECT ... FROM ... WHERE "some_tree" = (({1},{2}),({3},{4}))` (here `{1}` means `MyModel.some_tree.db_value(1))`. I don't know if `((1,2),(3,4))` is Ok in `WHERE` clause in PostgreSQL, but I've never seen constructions like this before..
Also it makes impossible to use iterables as python representation of db values at all.


Do you mind trying this out?

``` python
param = Param([(1, 2), (3, 4)])
print MyModel.select().where(MyModel.some_tree == param).sql()
```


@coleifer 

This does not convert param (1):

```
p = peewee.Param([1,2])
print MyModel.select().where(MyModel.some_tree == p).sql()

('SELECT t1."id", t1."some_value", t1."some_tree" FROM "mymodel" AS t1 WHERE (t1."some_tree" = %s)', [[1, 2]])
```

This does convert param (2):

```
p = peewee.Param(MyModel.some_tree.db_value([1,2]))
print MyModel.select().where(MyModel.some_tree == p).sql()

('SELECT t1."id", t1."some_value", t1."some_tree" FROM "mymodel" AS t1 WHERE (t1."some_tree" = %s)', ['000001.000002'])
```

This fails on trying to convert converted value (as expected) (3):     

```
p = MyModel.some_tree.db_value([1,2])
print MyModel.select().where(MyModel.some_tree == p).sql()
```

Yes, I found the working way for this query (2), but it looks too verbose.
`Param` is good and it works as expected in all cases (1-2).

But I still think that behavior of `.where(field == value)` is not evident. And this operator shall just cast `value` to `db_value` (and not iterate over it) as `.where(field << iterable)` shall iterate over `iterable` once and not recursive.
What do you think?


The reason why `.where(field << value)` is recursive is because it needs to handle the (fairly common) case of subqueries, i.e. `WHERE id IN (SELECT ...)`.

I'll look into this some more and see if there's a better fix, though.


@coleifer 
I think that the most sane way is to check types in every operator, which may (by design) handle pythonic value:
- subclass of `Node` -> handle like `Node`
- `iterable` in places where iterables are ok -> iterate
- everything else -> cast to db_value


Adding the context "where iterables are ok" is what concerns me about that plan.


@coleifer 

Ive made some monkey-patch draft for this problem.

```
# -*- coding: utf-8 -*-
from psycopg2ct import compat

compat.register()

import peewee


def patch():
    list_to_cast = ['__eq__']
    list_to_map = ['__lshift__']

    patched = {}

    def mutate_with_cast(op):
        original = peewee.Node.__dict__[op]

        def mutated(self, rhs):
            if not isinstance(rhs, peewee.Node):
                rhs = peewee.Param(self.db_value(rhs))
            return original(self, rhs)

        patched[op] = mutated

    def mutate_with_map(op):
        original = peewee.Node.__dict__[op]

        def mutated(self, rhs):
            rhs = [i if isinstance(i, peewee.Node) else peewee.Param(self.db_value(i)) for i in rhs]
            return original(self, rhs)

        patched[op] = mutated

    map(mutate_with_cast, list_to_cast)
    map(mutate_with_map, list_to_map)

    peewee.Node.__dict__.update(patched)


patch()

import playhouse.postgres_ext as pp


class IntLTreeField(peewee.Field):
    db_field = 'ltree'
    template = '%(column_type)s'

    def field_attributes(self):
        return {
            'length': 6,
        }

    def db_value(self, value):
        formatter = "{{:0{length:}}}".format(**self.attributes)
        return '.'.join(map(formatter.format, value))

    def python_value(self, value):
        return map(int, value.split('.'))


pp.PostgresqlExtDatabase.register_fields({'ltree': 'LTREE'})

db = pp.PostgresqlExtDatabase('mydb')


class MyModel(peewee.Model):
    class Meta:
        database = db

    some_value = peewee.IntegerField()
    some_tree = IntLTreeField(length=6)


print MyModel.select().where(MyModel.some_tree == [1, 2]).sql()
print MyModel.select().where(MyModel.some_tree << [[1, 2], [2, 3]]).sql()
```

Outputs:

```
 ('SELECT t1."id", t1."some_value", t1."some_tree" FROM "mymodel" AS t1 WHERE (t1."some_tree" = %s)', ['000001.000002'])
 ('SELECT t1."id", t1."some_value", t1."some_tree" FROM "mymodel" AS t1 WHERE (t1."some_tree" IN (%s, %s))', ['000001.000002', '000002.000003'])
```

Looks like it work. :)

.. but it needs to add each op into corresponding list to patch. 
.. and to rewrite some mappings depending on import style. 

What do you think?


That's definitely an approach I had not thought of.  I do have a concern, though, about changing the data at the time the expression is evaluated in python.  My preference w/the 2.0 rewrite has been to do these kinds of things in the `QueryCompiler`.  I understand that the way the `QueryCompiler` handles lists is not ideal for situations where you're trying to pass the list to `db_value`, however I'd rather not change the data at expression evaluation time.


Why not? If you compare 2 **complicated** values where one is a db-type, and other is a py-type, then one of them shall be casted to proper type (or at least to good type for query placeholders). 

The questions are: which of them shall be casted (A), and who is responsible for that (B)?

A. When we prepare query for db, it's obvious, that all shall be casted into db-values. When we getting something from db, we convert it into py-value. And not vice-versa.

B. We have (as i suppose) 4 places, where these values can be converted:
1) somewhere inside db or in query line;
2) somewhere inside peewee, when query is about to execute;
3) somewhere inside peewee, when query is building;
4) manually.

The 1-st is bad.Because we use db as data-storage and we try not to move any business-logic into it. Also, unreasonable complicated queries are not good. Ex:

Theese queries

```
SELECT * FROM my_table WHERE tree ~ "%s.%s.%s".%s".%s.%s" ::ltree
SELECT * FROM my_table WHERE tree ~ "%s.%s.%s".%s".%s" ::ltree
SELECT * FROM my_table WHERE tree ~ "%s.%s" ::ltree
```

are the same as this:

```
SELECT * FROM my_table WHERE tree ~ "%s" ::ltree 
```

The 2-nd is bad too, because we need to "remember" conversion functions for each field of each model involved into query. It complicates peewee's logic and brings lots of unnecessary overheads. 
The 3-rd is better, because we convert and 'freeze' parameters right at the place and time, when and where we build query.
The 4-th - Manual operations - ? No. 


So the crux of the problem is that peewee handles lists differently than other values.  You want to be able to pass the list unmodified into the `db_value()` method of the field.  There is a mechanism for that (`Param`) but it isn't doing the right thing -- I think the fix is to make `Param` use `db_value()` when possible.

What do you think?

``` diff
diff --git a/peewee.py b/peewee.py
index 2386dab..8ab17c0 100644
--- a/peewee.py
+++ b/peewee.py
@@ -45,6 +45,7 @@ __all__ = [
     'JOIN_LEFT_OUTER',
     'Model',
     'MySQLDatabase',
+    'Param',
     'PostgresqlDatabase',
     'prefetch',
     'PrimaryKeyField',
@@ -838,6 +839,7 @@ class QueryCompiler(object):
                 node.nodes, alias_map, conv, ' ')
         elif isinstance(node, Param):
             params = [node.value]
+            unknown = True
         elif isinstance(node, R):
             sql = node.value
             params = []
```

The way you would use it would be:

``` python
from peewee import *

class IntLTreeField(Field):
    db_field = 'ltree'
    template = '%(column_type)s'

    def field_attributes(self):
        return {'length': 6}

    def db_value(self, value):
        formatter = "{{:0{length:}}}".format(**self.attributes)
        return '.'.join(map(formatter.format, value))

    def python_value(self, value):
        return map(int, value.split('.'))

class MyModel(Model):
    some_tree = IntLTreeField()

print MyModel.select().where(MyModel.some_tree == Param([1, 2])).sql()
trees = [
    Param([1, 2]),
    Param([3, 4])]
print MyModel.select().where(MyModel.some_tree << trees).sql()
```

Prints:
('SELECT t1."id", t1."some_tree" FROM "mymodel" AS t1 WHERE (t1."some_tree" = ?)', ['000001.000002'])
('SELECT t1."id", t1."some_tree" FROM "mymodel" AS t1 WHERE (t1."some_tree" IN (?, ?))', ['000001.000002', '000003.000004'])


Added in 51c949c7a7d5f8969ef77611c2c8016b692df00b


I like it!


Cool, thanks for talking this one through w/me!