From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 15 Aug 2014 00:00:35 +0000 (-0400)
Subject: - The string keys that are used to determine the columns impacted
X-Git-Tag: rel_1_0_0b1~230
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6a21f9e328361d5185fd616e7992a183030f9a10;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- 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 a797bfa295..0f92fb254c 100644
--- 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 2dc4d43f2d..65753b6dcf 100644
--- 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 291aee2f34..f65168552a 100644
--- a/test/engine/test_execute.py
+++ b/test/engine/test_execute.py
@@ -688,6 +688,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()
 
@@ -705,12 +706,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):