consider existing bindparam()s in an INSERT/UPDATE not part of column_keys but are named after a column?
i.e.
from sqlalchemy import *
from sqlalchemy.sql import table, column
t = table('foo', column('x'), column('y'))
u = t.update().where(t.c.x==bindparam('x'))
print u.compile(column_keys=['y']('x',))
would produce:
UPDATE foo SET y=:y WHERE foo.x = :x
this is actually tricky since visit_update() handles the SET clause before the WHERE. Reversing the order of those so that we know what was named in the WHERE then breaks the bind param ordering on positional bind sets. There's ways to get around this but all of which would complicate the internals of visit_update().
Comments (9)
-
reporter -
reporter - changed component to sql
we're going to go with exceptions. this is hard.
here's a patch. consider moving the check to be unconditional, since as it is, it allows a
compile()
to occur, but not anycompile()
within anexecute()
that actually sets up those bind parameter names, so its sort of useless.Index: lib/sqlalchemy/sql/compiler.py =================================================================== --- lib/sqlalchemy/sql/compiler.py (revision 6427) +++ lib/sqlalchemy/sql/compiler.py (working copy) @@ -475,12 +475,19 @@ def visit_bindparam(self, bindparam, **kwargs): name = self._truncate_bindparam(bindparam) + if name in self.binds: + + if (self.isinsert or self.isupdate) and ((self.column_keys and name in self.column_keys) \ + or (not self.statement.parameters and name in self.statement.table.c)): + raise exc.CompileError("XXXFIXME This will be an error message referring to your usage of bind parameter '%s' during INSERT or UPDATE." % bindparam.key) + existing = self.binds[name](name) if existing is not bindparam and (existing.unique or bindparam.unique): raise exc.CompileError( "Bind parameter '%s' conflicts with unique bind parameter of the same name" % bindparam.key ) + self.binds[bindparam.key](bindparam.key) = self.binds[name](name) = bindparam return self.bindparam_string(name) @@ -796,7 +803,9 @@ if stmt.parameters is not None: for k, v in stmt.parameters.iteritems(): - parameters.setdefault(sql._column_as_key(k), v) + v2 = parameters.setdefault(sql._column_as_key(k), v) + if v2 is not v and not sql._is_literal(v): + raise exc.CompileError("Expression '%s' would be replaced by execution-time literal" % v) # create a list of column assignment clauses as tuples values = [things to test: {{{ from sqlalchemy import * from sqlalchemy.sql import table, column t = table('foo', column('x'), column('y')) u = t.update().where(t.c.x==bindparam('x')) print u.compile() print u.values(x=7).compile() print u.values(y=7).compile() print u.values(x=7).compile(column_keys=['x', 'y'](]
here's)) print u.values(x=3 + bindparam('x')).compile(column_keys='y') print u.compile(column_keys='y')
i = t.insert().values(x=3 + bindparam('x')) print i.compile() print i.compile(column_keys='y')
i = t.insert().values(x=3 + bindparam('x2')) print i.compile(column_keys=[i.compile() print i.compile(column_keys='x', 'y' print)) print i.compile(column_keys='y')
}}}
-
reporter - marked as critical
-
reporter - changed title to consider existing bindparam()s in an INSERT/UPDATE not part of column_keys but are named after a column?
-
reporter OK, here's another patch that costs us 5 function calls on the update/insert tests, doesn't blow up zoomark, allows all those tests to work in a nonsurprising way:
Index: lib/sqlalchemy/sql/compiler.py =================================================================== --- lib/sqlalchemy/sql/compiler.py (revision 6427) +++ lib/sqlalchemy/sql/compiler.py (working copy) @@ -776,12 +776,17 @@ self.prefetch = [ self.returning = [](] ) + bind_names = set() + visitors.traverse(stmt, {}, {'bindparam':lambda x:bind_names.add(x.key)}) + if stmt.parameters: + bind_names.update(stmt.parameters) + # no parameters in the statement, no parameters in the # compiled params - return binds for all columns if self.column_keys is None and stmt.parameters is None: return [ (c, self._create_crud_bind_param(c, None, required=True)) - for c in stmt.table.columns + for c in stmt.table.columns if c.key not in bind_names ]( ) required = object() @@ -792,7 +797,7 @@ parameters = {} else: parameters = dict((sql._column_as_key(key), required) - for key in self.column_keys) + for key in self.column_keys if key not in bind_names) if stmt.parameters is not None: for k, v in stmt.parameters.iteritems():
-
reporter - changed status to resolved
the latter worked out really well. a43a0e8b68c28b72404e85e4b4e8999443dd3fc5.
-
reporter - changed status to open
- removed status
Reopening for two reasons.
- we need 5+ method calls for every insert() and update() even though this is an extremely unusual case ?
-
it doesn't work completely:
table with "x" and "y":
t.insert().values(x=bindparam('y'), y=5)
produces:
INSERT INTO foo (x, y) VALUES (:y, :y)
the "raise an error" approach is fine. the compilation is impacted by the specific columns you name in execute(), but so what - those are still structural and not data driven. otherwise you can get unpredictable behavior as above.
-
reporter - changed status to resolved
most of those test cases are still possible - just a few more, including the flat out wrong ones, now raise in ebe7f7b15e03a30ae14263f16fed8a18c35eebd9.
-
reporter - removed milestone
Removing milestone: 0.6.0 (automated comment)
- Log in to comment
Otherwise consider adding an exception when a bindparam() is detected with a column name as a key.