From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 9 Feb 2017 02:42:34 +0000 (-0500)
Subject: Don't post-SELECT columns w/o a server default/onupdate for eager_defaults
X-Git-Tag: rel_1_1_6~11
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=000e9603065edeb997cfcb492c4063029a931de9;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Don't post-SELECT columns w/o a server default/onupdate for eager_defaults

Fixed a major inefficiency in the "eager_defaults" feature whereby
an unnecessary SELECT would be emitted for column values where the
ORM had explicitly inserted NULL, corresponding to attributes that
were unset on the object but did not have any server default
specified, as well as expired attributes on update that nevertheless
had no server onupdate set up.   As these columns are not part of the
RETURNING that eager_defaults tries to use, they should not be
post-SELECTed either.

Change-Id: I0d4f1e9d3d9717d68dcc0592f69456a1f1c36df8
Fixes: #3909
---

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 9b22a123c2..d4b95e6ee1 100644
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -21,6 +21,19 @@
 .. changelog::
     :version: 1.1.6
 
+    .. change:: 3909
+        :tags: bug, orm
+        :tickets: 3909
+
+        Fixed a major inefficiency in the "eager_defaults" feature whereby
+        an unnecessary SELECT would be emitted for column values where the
+        ORM had explicitly inserted NULL, corresponding to attributes that
+        were unset on the object but did not have any server default
+        specified, as well as expired attributes on update that nevertheless
+        had no server onupdate set up.   As these columns are not part of the
+        RETURNING that eager_defaults tries to use, they should not be
+        post-SELECTed either.
+
     .. change:: 3905
         :tags: bug, sql
         :tickets: 3905
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index f45b56ae71..962486d581 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -2041,6 +2041,22 @@ class Mapper(InspectionAttr):
             for table, columns in self._cols_by_table.items()
         )
 
+    @_memoized_configured_property
+    def _server_default_plus_onupdate_propkeys(self):
+        result = set()
+
+        for table, columns in self._cols_by_table.items():
+            for col in columns:
+                if (
+                        (
+                            col.server_default is not None or
+                            col.server_onupdate is not None
+                        ) and col in self._columntoproperty
+                ):
+                    result.add(self._columntoproperty[col].key)
+
+        return result
+
     @_memoized_configured_property
     def _server_onupdate_default_cols(self):
         return dict(
diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 8f3e5de584..4d1e38d3f6 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -994,8 +994,12 @@ def _finalize_insert_update_commands(base_mapper, uowtransaction, states):
         toload_now = []
 
         if base_mapper.eager_defaults:
-            toload_now.extend(state._unloaded_non_object)
-        elif mapper.version_id_col is not None and \
+            toload_now.extend(
+                state._unloaded_non_object.intersection(
+                    mapper._server_default_plus_onupdate_propkeys)
+            )
+
+        if mapper.version_id_col is not None and \
                 mapper.version_id_generator is False:
             if mapper._version_id_prop.key in state.unloaded:
                 toload_now.extend([mapper._version_id_prop.key])
diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py
index aec3a4d071..ecd3650003 100644
--- a/test/orm/test_unitofworkv2.py
+++ b/test/orm/test_unitofworkv2.py
@@ -2322,6 +2322,46 @@ class EagerDefaultsTest(fixtures.MappedTest):
             )
         )
 
+    def test_insert_dont_fetch_nondefaults(self):
+        Thing2 = self.classes.Thing2
+        s = Session()
+
+        t1 = Thing2(id=1, bar=2)
+
+        s.add(t1)
+
+        self.assert_sql_execution(
+            testing.db,
+            s.flush,
+            CompiledSQL(
+                "INSERT INTO test2 (id, foo, bar) "
+                "VALUES (:id, :foo, :bar)",
+                [{'id': 1, 'foo': None, 'bar': 2}]
+            )
+        )
+
+    def test_update_dont_fetch_nondefaults(self):
+        Thing2 = self.classes.Thing2
+        s = Session()
+
+        t1 = Thing2(id=1, bar=2)
+
+        s.add(t1)
+        s.flush()
+
+        s.expire(t1, ['foo'])
+
+        t1.bar = 3
+
+        self.assert_sql_execution(
+            testing.db,
+            s.flush,
+            CompiledSQL(
+                "UPDATE test2 SET bar=:bar WHERE test2.id = :test2_id",
+                [{'bar': 3, 'test2_id': 1}]
+            )
+        )
+
 
 class TypeWoBoolTest(fixtures.MappedTest, testing.AssertsExecutionResults):
     """test support for custom datatypes that return a non-__bool__ value