Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

can't use expanding IN parameter more than once #4394

Closed
zzzeek opened this issue Dec 2, 2018 · 4 comments
Closed

can't use expanding IN parameter more than once #4394

zzzeek opened this issue Dec 2, 2018 · 4 comments
Labels
bug Something isn't working high priority sql
Milestone

Comments

@zzzeek
Copy link
Member

zzzeek commented Dec 2, 2018

I'm not sure of the rationale of using "pop" at https://github.com/sqlalchemy/sqlalchemy/blob/master/lib/sqlalchemy/engine/default.py#L735 for positional and https://github.com/sqlalchemy/sqlalchemy/blob/master/lib/sqlalchemy/engine/default.py#L785 for named; both fail when the same parameter is used more than once, which should be supported:

from sqlalchemy import *
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy import exc
import traceback

Base = declarative_base()


class A(Base):
    __tablename__ = 'a'

    id = Column(Integer, primary_key=True)


bind = bindparam('id', expanding=True)
stmt = select([A]).where(A.id.in_(bind) | A.id.in_(bind))

e1 = create_engine("sqlite://")
e2 = create_engine("postgresql://scott:tiger@localhost/test")

Base.metadata.create_all(e1)
Base.metadata.create_all(e2)

try:
    e1.execute(stmt, id=[1, 2, 3])
except exc.StatementError as err:
    traceback.print_exc()

try:
    e2.execute(stmt, id=[1, 2, 3])
except exc.StatementError as err:
    traceback.print_exc()

tracebacks:

  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/engine/default.py", line 656, in _init_compiled
    positiontup = self._expand_in_parameters(compiled, processors)
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/engine/default.py", line 735, in _expand_in_parameters
    values = compiled_params.pop(name)
sqlalchemy.exc.StatementError: (builtins.KeyError) 'id' [SQL: 'SELECT a.id \nFROM a \nWHERE a.id IN ([EXPANDING_id]) OR a.id IN ([EXPANDING_id])'] [parameters: [{'id': [1, 2, 3]}]]

######################

  File "/home/classic/.venv3/lib/python3.7/re.py", line 192, in sub
    return _compile(pattern, flags).sub(repl, string, count)
  File "/home/classic/dev/sqlalchemy/lib/sqlalchemy/engine/default.py", line 781, in process_expanding
    return replacement_expressions.pop(m.group(1))
sqlalchemy.exc.StatementError: (builtins.KeyError) 'id' [SQL: 'SELECT a.id \nFROM a \nWHERE a.id IN ([EXPANDING_id]) OR a.id IN ([EXPANDING_id])'] [parameters: [{'id': [1, 2, 3]}]]


I need to understand why these might have been pops- both can be changed to straight index access to fix and tests pass:

diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py
index 915812a4f2..6f53e22564 100644
--- a/lib/sqlalchemy/engine/default.py
+++ b/lib/sqlalchemy/engine/default.py
@@ -732,7 +732,7 @@ class DefaultExecutionContext(interfaces.ExecutionContext):
         ):
             parameter = self.compiled.binds[name]
             if parameter.expanding:
-                values = compiled_params.pop(name)
+                values = compiled_params[name]
                 if not values:
                     raise exc.InvalidRequestError(
                         "'expanding' parameters can't be used with an "
@@ -778,7 +778,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+)\]",
@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue:

Maintain compiled_params / replacement_expressions within expanding IN https://gerrit.sqlalchemy.org/1051

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue:

Maintain compiled_params / replacement_expressions within expanding IN https://gerrit.sqlalchemy.org/1052

@zzzeek
Copy link
Member Author

zzzeek commented Dec 22, 2018

OK yeah that approach doesn't work. We are replacing param[name] with the expanded set, it is unique. if we have two occurrences of "name", we need two expanded sets. not sure how the test case is working.

@zzzeek
Copy link
Member Author

zzzeek commented Dec 22, 2018

hmmm but the name is the same, so it should be the same. HMMM. OK

sqlalchemy-bot pushed a commit that referenced this issue Dec 22, 2018
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
(cherry picked from commit c495769)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working high priority sql
Projects
None yet
Development

No branches or pull requests

2 participants