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 9b22a123c292e5f3eeea092429e473e1094a01e0..d4b95e6ee124510feb4850dbba17600ad3c391af 100644 (file)
--- 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 f45b56ae71d9479cc23a404126cc6378ec004b08..962486d581b34cab9106a46f1d84e82f45dbf158 100644 (file)
--- 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 8f3e5de5844fc7901b333523fff0b80dde12f840..4d1e38d3f6ba593805b7fa7959c0a2868b60dd10 100644 (file)
--- 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 aec3a4d07103eafa6c22766be69aacccabe2147c..ecd3650003db23b0e2b82cb833df6bf396a54236 100644 (file)
--- 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