Add support of empty list in exanding of bindparam

[NicolasRolin]

Add the support of empty lists to expanding_in of bindparam, by replacing the list by a query that should do an empty list in reasonable DB : "SELECT 1 FROM ((SELECT 1) as placeholder_table) WHERE 1!=1;"
See https://groups.google.com/forum/#!topic/sqlalchemy/NLJCu_vqK4g for the discussion.

My tests in local passes, and if I understand it well, it means I only tested for sqlite. Should work for PG and mysql too, but I can't be sure for other dbs.

This is my first contribution, so I guess I did a lot of mistakes, sorry.

[zzzeek]

SQLite supports empty in directly:

sqlite> select 1 where 1 in ();

probably why the test passed

[zzzeek]

shouldn't "key" be "value" here? but also, we're trying to embed a fixed SQL expression so we don't really need a whole iteration with a to_update loop ?

[NicolasRolin]

I didn't master this part of the code, so I imitated the non-empty case (https://github.com/zzzeek/sqlalchemy/blob/master/lib/sqlalchemy/engine/default.py#L763) to inject the query instead of a list. I guess there is a better way, but by doing it this way I was sure to be consistent with the other parts of the if/else and get the string repalcement I was hoping to do.

[zzzeek]

it does not work, because it does not render the SQL, it's just injecting it as the parameter:

from sqlalchemy import table, column, select, bindparam
from sqlalchemy import create_engine

t1 = table('t1', column('x'))
stmt = select([t1]).where(t1.c.x.in_(bindparam('q', expanding=True)))

e = create_engine("sqlite://", echo=True)
e.execute("create table t1 (x integer)")
e.execute(stmt, {"q": []})

output:

SELECT t1.x 
FROM t1 
WHERE t1.x IN (?)
2018-03-12 11:22:54,111 INFO sqlalchemy.engine.base.Engine ('SELECT 1 FROM (SELECT 1) as placeholder_table WHERE 1!=1',)

here's postgresql:

sqlalchemy.exc.DataError: (psycopg2.DataError) invalid input syntax for integer: "SELECT 1 FROM (SELECT 1) as placeholder_table WHERE 1!=1"
LINE 3: WHERE t1.x IN ('SELECT 1 FROM (SELECT 1) as placeholder_tabl...
                       ^
 [SQL: 'SELECT t1.x \nFROM t1 \nWHERE t1.x IN (%(q_1)s)'] [parameters: {'q_1': 'SELECT 1 FROM (SELECT 1) as placeholder_table WHERE 1!=1'}] 

[zzzeek]

why not use a select() construct here that way we aren't hardcoding string SQL outside of compiler.py

[NicolasRolin]

I guess the corresponding select is stmt = "select([1]).select_from(select([1]).alias('placeholer')).where(1!=1)".
Shoud I replace my EMPTY_SET_QUERY by
stmt.__str__() ?

[NicolasRolin]

I have some trouble using the select construct to give me a good empty set query.

from sqlalchemy import table, column, select, bindparam
from sqlalchemy import create_engine

empty_set_query = select([1]).select_from(
    select([1]).alias('placeholer')
).where(1 != 1)
stmt = "SELECT NULL IN (%s)" % empty_set_query

e = create_engine('sqlite://', echo=False)
e.execute(stmt).fetchone()

output:
OperationalError: (sqlite3.OperationalError) no such column: false [SQL: 'SELECT NULL IN (SELECT 1 \nFROM (SELECT 1) AS placeholer \nWHERE false)']

stmt = 'SELECT NULL IN (SELECT 1 \nFROM (SELECT 1) AS placeholer \nWHERE 1!=1)'
e.execute(stmt).fetchone()

output:
(0,)

So it seems that my query fails because "1!=1" is not equal "false". Is there a way to get a compiled query with a "1!=1" ?

[NicolasRolin]

This use a select construct as suggested, and my tests passes in local. Is there anything else that needs to be fixed ?

[zzzeek]

There's a lot of things I'd want to do with this.

The first thing we need to do is get it in gerrit, so there are some code things to clean up:

1. definitely cannot call str() on a SQL expression, that bypasses all dialect-level hooks. at best you can do expr.compile(dialect=dialect)

2. abstract away the concept of "empty set expression" into something the dialects can give us from their Compiler subclasses, since I'm sure in cases like Oracle, MSSQL there are going to be problems, e.g. like compiler.visit_empty_set_expr()

3. needs dialect-level tests under test/dialect/test_suite, in lib/sqlalchemy/testing/suite/test_select.py->ExpandingBoundInTest. these need to test not only the empty set condition but also the various negation use cases. when the test is in there, it will run against every DB on the continuous integration servers.

then things to get it going, which I usually do:

1. bug report so this can be tracked, targeted at 1.3

2. will need migration notes in migration_13.rst, etc.

[NicolasRolin]

Ok, now I call the dialect compiler to have access to the empty set query (albeit atm is just returns the same string no matter the dialect as I have no idea of who support it).
I added tests in the testing suite for the in_([]) and notin_([]).

[zzzeek]

I can pull this into gerrit (which runs the tests) where we will have to make changes to accommodate for Oracle and maybe SQL Server. Can I get you to participate on that? or would you prefer I work on it later for 1.3 ?

[NicolasRolin]

I would be glad to participe, even if I might not be too usefull as I have no understanding of the internals of sqlalchemy :)

[zzzeek]

the bigger challenge is using gerrit if you haven't used it before.

[zzzeek]

Dear contributor -

This pull request is being moved to Gerrit, at https://gerrit.sqlalchemy.org/#/c/zzzeek/sqlalchemy/+/760, where it may be tested and reviewed more closely. As such, the pull request itself is being marked "closed" or "declined", however your contribution is merely being moved to our central review system. Please register at https://gerrit.sqlalchemy.org#/register/ to send and receive comments regarding this item.

[zzzeek]

if you register over there you can see the failures so far. Postgresql is failing first. it won't run more backends until it can get through Postgresql.