From c495769751e8b19d54fb92388ced587b5d13b85d Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 21 Dec 2018 17:35:12 -0500 Subject: [PATCH] Maintain compiled_params / replacement_expressions within expanding IN Fixed issue in "expanding IN" feature where using the same bound parameter name more than once in a query would lead to a KeyError within the process of rewriting the parameters in the query. Fixes: #4394 Change-Id: Ibcadce9fefbcb060266d9447c2044ee6efeccf5a --- doc/build/changelog/unreleased_12/4394.rst | 7 ++ lib/sqlalchemy/engine/default.py | 82 +++++++++++++--------- test/sql/test_query.py | 35 +++++++++ 3 files changed, 90 insertions(+), 34 deletions(-) create mode 100644 doc/build/changelog/unreleased_12/4394.rst diff --git a/doc/build/changelog/unreleased_12/4394.rst b/doc/build/changelog/unreleased_12/4394.rst new file mode 100644 index 0000000000..faa3547ad0 --- /dev/null +++ b/doc/build/changelog/unreleased_12/4394.rst @@ -0,0 +1,7 @@ +.. change:: + :tag: bug, sql + :tickets: 4394 + + Fixed issue in "expanding IN" feature where using the same bound parameter + name more than once in a query would lead to a KeyError within the process + of rewriting the parameters in the query. diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py index 5c96e4240e..028abc4c24 100644 --- a/lib/sqlalchemy/engine/default.py +++ b/lib/sqlalchemy/engine/default.py @@ -726,50 +726,64 @@ class DefaultExecutionContext(interfaces.ExecutionContext): positiontup = None replacement_expressions = {} + to_update_sets = {} + for name in ( self.compiled.positiontup if compiled.positional else self.compiled.binds ): parameter = self.compiled.binds[name] if parameter.expanding: - values = compiled_params.pop(name) - if not values: - to_update = [] - replacement_expressions[name] = ( - self.compiled.visit_empty_set_expr( - parameter._expanding_in_types - if parameter._expanding_in_types - else [parameter.type] + + if name in replacement_expressions: + to_update = to_update_sets[name] + else: + # we are removing the parameter from compiled_params + # because it is a list value, which is not expected by + # TypeEngine objects that would otherwise be asked to + # process it. the single name is being replaced with + # individual numbered parameters for each value in the + # param. + values = compiled_params.pop(name) + + if not values: + to_update = to_update_sets[name] = [] + replacement_expressions[name] = ( + self.compiled.visit_empty_set_expr( + parameter._expanding_in_types + if parameter._expanding_in_types + else [parameter.type] + ) ) - ) - elif isinstance(values[0], (tuple, list)): - to_update = [ - ("%s_%s_%s" % (name, i, j), value) - for i, tuple_element in enumerate(values, 1) - for j, value in enumerate(tuple_element, 1) - ] - replacement_expressions[name] = ", ".join( - "(%s)" % ", ".join( + elif isinstance(values[0], (tuple, list)): + to_update = to_update_sets[name] = [ + ("%s_%s_%s" % (name, i, j), value) + for i, tuple_element in enumerate(values, 1) + for j, value in enumerate(tuple_element, 1) + ] + replacement_expressions[name] = ", ".join( + "(%s)" % ", ".join( + self.compiled.bindtemplate % { + "name": + to_update[i * len(tuple_element) + j][0] + } + for j, value in enumerate(tuple_element) + ) + for i, tuple_element in enumerate(values) + + ) + else: + to_update = to_update_sets[name] = [ + ("%s_%s" % (name, i), value) + for i, value in enumerate(values, 1) + ] + replacement_expressions[name] = ", ".join( self.compiled.bindtemplate % { - "name": - to_update[i * len(tuple_element) + j][0] - } - for j, value in enumerate(tuple_element) + "name": key} + for key, value in to_update ) - for i, tuple_element in enumerate(values) - ) - else: - to_update = [ - ("%s_%s" % (name, i), value) - for i, value in enumerate(values, 1) - ] - replacement_expressions[name] = ", ".join( - self.compiled.bindtemplate % { - "name": key} - for key, value in to_update - ) compiled_params.update(to_update) processors.update( (key, processors[name]) @@ -783,7 +797,7 @@ class DefaultExecutionContext(interfaces.ExecutionContext): positiontup.append(name) def process_expanding(m): - return replacement_expressions.pop(m.group(1)) + return replacement_expressions[m.group(1)] self.statement = re.sub( r"\[EXPANDING_(\S+)\]", diff --git a/test/sql/test_query.py b/test/sql/test_query.py index 971374eb92..175b69c4f2 100644 --- a/test/sql/test_query.py +++ b/test/sql/test_query.py @@ -529,6 +529,41 @@ class QueryTest(fixtures.TestBase): [(8, 'fred'), (9, 'ed')] ) + def test_expanding_in_repeated(self): + testing.db.execute( + users.insert(), + [ + dict(user_id=7, user_name='jack'), + dict(user_id=8, user_name='fred'), + dict(user_id=9, user_name='ed') + ] + ) + + with testing.db.connect() as conn: + stmt = select([users]).where( + users.c.user_name.in_( + bindparam('uname', expanding=True) + ) | users.c.user_name.in_(bindparam('uname2', expanding=True)) + ).where(users.c.user_id == 8) + stmt = stmt.union( + select([users]).where( + users.c.user_name.in_( + bindparam('uname', expanding=True) + ) | users.c.user_name.in_( + bindparam('uname2', expanding=True)) + ).where(users.c.user_id == 9) + ).order_by(stmt.c.user_id) + + eq_( + conn.execute( + stmt, + { + "uname": ['jack', 'fred'], + "uname2": ['ed'], "userid": [8, 9]} + ).fetchall(), + [(8, 'fred'), (9, 'ed')] + ) + @testing.requires.tuple_in def test_expanding_in_composite(self): testing.db.execute( -- 2.47.2