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

Expanding bindparam not running bind processors #4198

Closed
sqlalchemy-bot opened this issue Feb 22, 2018 · 9 comments
Closed

Expanding bindparam not running bind processors #4198

sqlalchemy-bot opened this issue Feb 22, 2018 · 9 comments
Labels
bug Something isn't working high priority sql
Milestone

Comments

@sqlalchemy-bot
Copy link
Collaborator

Migrated issue, originally created by Robert Ditthardt

I'm assuming there are just missing features here because experimental. That said...

Attached is a test file that demonstrates the observed behavior.

I think that it should be unnecessary to need to get the "name" attribute of an enumeration for "in_" queries that require bound parameters that are enumerations. It should be sufficient to just pass in the enumeration as it is when the Table is properly configured.

I should also note that the mysql driver has slightly different behavior. It doesn't throw an exception, but instead just silently generates a bad query, where each enum in the raw sql ends up as "TestEnum.FOO" instead of "FOO".


Attachments: test-sqlalchemy.py

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

bind param processing logic entirely non functional. wrote the code to process parameters, can take it right out and all the tests pass. here's generalized test:

diff --git a/test/sql/test_types.py b/test/sql/test_types.py
index 002094f7b..29746686a 100644
--- a/test/sql/test_types.py
+++ b/test/sql/test_types.py
@@ -303,17 +303,22 @@ class UserDefinedTest(fixtures.TablesTest, AssertsCompiledSQL):
 
     """tests user-defined types."""
 
+    def _data_fixture(self):
+        users = self.tables.users
+        with testing.db.connect() as conn:
+            conn.execute(users.insert(), dict(
+                user_id=2, goofy='jack', goofy2='jack', goofy4=util.u('jack'),
+                goofy7=util.u('jack'), goofy8=12, goofy9=12))
+            conn.execute(users.insert(), dict(
+                user_id=3, goofy='lala', goofy2='lala', goofy4=util.u('lala'),
+                goofy7=util.u('lala'), goofy8=15, goofy9=15))
+            conn.execute(users.insert(), dict(
+                user_id=4, goofy='fred', goofy2='fred', goofy4=util.u('fred'),
+                goofy7=util.u('fred'), goofy8=9, goofy9=9))
+
     def test_processing(self):
         users = self.tables.users
-        users.insert().execute(
-            user_id=2, goofy='jack', goofy2='jack', goofy4=util.u('jack'),
-            goofy7=util.u('jack'), goofy8=12, goofy9=12)
-        users.insert().execute(
-            user_id=3, goofy='lala', goofy2='lala', goofy4=util.u('lala'),
-            goofy7=util.u('lala'), goofy8=15, goofy9=15)
-        users.insert().execute(
-            user_id=4, goofy='fred', goofy2='fred', goofy4=util.u('fred'),
-            goofy7=util.u('fred'), goofy8=9, goofy9=9)
+        self._data_fixture()
 
         result = users.select().order_by(users.c.user_id).execute().fetchall()
         for assertstr, assertint, assertint2, row in zip(
@@ -331,6 +336,27 @@ class UserDefinedTest(fixtures.TablesTest, AssertsCompiledSQL):
             for col in row[3], row[4]:
                 assert isinstance(col, util.text_type)
 
+    def test_plain_in(self):
+        users = self.tables.users
+        self._data_fixture()
+
+        stmt = select([users.c.user_id, users.c.goofy8]).where(
+                users.c.goofy8.in_([15, 9])
+            )
+        result = testing.db.execute(stmt, {"goofy": [15, 9]})
+        eq_(result.fetchall(), [(3, 1500), (4, 900)])
+
+    def test_expanding_in(self):
+        users = self.tables.users
+        self._data_fixture()
+
+        stmt = select([users.c.user_id, users.c.goofy8]).where(
+                users.c.goofy8.in_(bindparam("goofy", expanding=True))
+            )
+        result = testing.db.execute(stmt, {"goofy": [15, 9]})
+        eq_(result.fetchall(), [(3, 1500), (4, 900)])
+        # MARKMARK
+
     def test_typedecorator_literal_render(self):
         class MyType(types.TypeDecorator):
             impl = String

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed title from "Expanding bindparam does not support Enums" to "Expanding bindparam not running bind processors"

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • added labels: sql

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • set milestone to "1.2.x"

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • added labels: high priority

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

and there you have it

diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py
index ed2ed0509..5806fe2a9 100644
--- a/lib/sqlalchemy/engine/default.py
+++ b/lib/sqlalchemy/engine/default.py
@@ -766,7 +766,7 @@ class DefaultExecutionContext(interfaces.ExecutionContext):
                 compiled_params.update(to_update)
                 processors.update(
                     (key, processors[name])
-                    for key in to_update if name in processors
+                    for key, value in to_update if name in processors
                 )
                 if compiled.positional:
                     positiontup.extend(name for name, value in to_update)

@sqlalchemy-bot
Copy link
Collaborator Author

@sqlalchemy-bot
Copy link
Collaborator Author

Michael Bayer (@zzzeek) wrote:

Allow bind processors to work with expanding IN

Fixed bug in new "expanding IN parameter" feature where the bind parameter
processors for values wasn't working at all, tests failed to cover this
pretty basic case which includes that ENUM values weren't working.

Change-Id: I8e2420d7229a3e253e43b5227ebb98f9fe0bd14a
Fixes: #4198

d746ea9

@sqlalchemy-bot
Copy link
Collaborator Author

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

@sqlalchemy-bot sqlalchemy-bot added high priority bug Something isn't working sql labels Nov 27, 2018
@sqlalchemy-bot sqlalchemy-bot added this to the 1.2.x milestone Nov 27, 2018
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

1 participant