- The string keys that are used to determine the columns impacted

for an INSERT or UPDATE are now sorted when they contribute towards
the "compiled cache" cache key.   These keys were previously not
deterministically ordered, meaning the same statement could be
cached multiple times on equivalent keys, costing both in terms of
memory as well as performance.
fixes #3165

diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst
index a797bfa295e39b57bf6784d3b97f8a8a15b18b07..0f92fb254c549593a4b23bec415e37abb47cae73 100644 (file)
--- a/doc/build/changelog/changelog_09.rst
+++ b/doc/build/changelog/changelog_09.rst
@@ -13,6 +13,18 @@
 .. changelog::
     :version: 0.9.8
 
+    .. change::
+        :tags: bug, engine
+        :versions: 1.0.0
+        :tickets: 3165
+
+        The string keys that are used to determine the columns impacted
+        for an INSERT or UPDATE are now sorted when they contribute towards
+        the "compiled cache" cache key.   These keys were previously not
+        deterministically ordered, meaning the same statement could be
+        cached multiple times on equivalent keys, costing both in terms of
+        memory as well as performance.
+
     .. change::
         :tags: bug, postgresql
         :versions: 1.0.0

diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py
index 57943a510e17fc0c95c552389c882cb9fc7acc82..7b7313a2b1de793530e53da78a369e549fe8fada 100644 (file)
--- a/lib/sqlalchemy/engine/base.py
+++ b/lib/sqlalchemy/engine/base.py
@@ -805,7 +805,7 @@ class Connection(Connectable):
 
         dialect = self.dialect
         if 'compiled_cache' in self._execution_options:
-            key = dialect, elem, tuple(keys), len(distilled_params) > 1
+            key = dialect, elem, tuple(sorted(keys)), len(distilled_params) > 1
             if key in self._execution_options['compiled_cache']:
                 compiled_sql = self._execution_options['compiled_cache'][key]
             else:

diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py
index c757e71ed08377d56c830cd6b9d3b459b4ec4482..675d22f17aa3249bf12604923ea5a2be6485e9c8 100644 (file)
--- a/test/engine/test_execute.py
+++ b/test/engine/test_execute.py
@@ -687,6 +687,7 @@ class CompiledCacheTest(fixtures.TestBase):
                       Column('user_id', INT, primary_key=True,
                              test_needs_autoincrement=True),
                       Column('user_name', VARCHAR(20)),
+                      Column("extra_data", VARCHAR(20))
                       )
         metadata.create_all()
 
@@ -704,12 +705,53 @@ class CompiledCacheTest(fixtures.TestBase):
         cached_conn = conn.execution_options(compiled_cache=cache)
 
         ins = users.insert()
-        cached_conn.execute(ins, {'user_name': 'u1'})
-        cached_conn.execute(ins, {'user_name': 'u2'})
-        cached_conn.execute(ins, {'user_name': 'u3'})
+        with patch.object(
+            ins, "compile",
+                Mock(side_effect=ins.compile)) as compile_mock:
+            cached_conn.execute(ins, {'user_name': 'u1'})
+            cached_conn.execute(ins, {'user_name': 'u2'})
+            cached_conn.execute(ins, {'user_name': 'u3'})
+        eq_(compile_mock.call_count, 1)
         assert len(cache) == 1
         eq_(conn.execute("select count(*) from users").scalar(), 3)
 
+    def test_keys_independent_of_ordering(self):
+        conn = testing.db.connect()
+        conn.execute(
+            users.insert(),
+            {"user_id": 1, "user_name": "u1", "extra_data": "e1"})
+        cache = {}
+        cached_conn = conn.execution_options(compiled_cache=cache)
+
+        upd = users.update().where(users.c.user_id == bindparam("b_user_id"))
+
+        with patch.object(
+            upd, "compile",
+                Mock(side_effect=upd.compile)) as compile_mock:
+            cached_conn.execute(
+                upd, util.OrderedDict([
+                    ("b_user_id", 1),
+                    ("user_name", "u2"),
+                    ("extra_data", "e2")
+                ])
+            )
+            cached_conn.execute(
+                upd, util.OrderedDict([
+                    ("b_user_id", 1),
+                    ("extra_data", "e3"),
+                    ("user_name", "u3"),
+                ])
+            )
+            cached_conn.execute(
+                upd, util.OrderedDict([
+                    ("extra_data", "e4"),
+                    ("user_name", "u4"),
+                    ("b_user_id", 1),
+                ])
+            )
+        eq_(compile_mock.call_count, 1)
+        eq_(len(cache), 1)
+
 
 class MockStrategyTest(fixtures.TestBase):