use load_scalar_attributes() for undefer

At some point, undeferral of attributes started emitting
a full ORM query for the entity, including that subclass columns
would use a JOIN.   Seems to be at least in 1.3 / 1.4 and possibly
earlier.    This JOINs to the superclass table unnecessarily.
Use load_scalar_attributes() here which should handle the whole
thing and emits a more efficient query for joined inheritance.

As this behavior seems to have been throughout 1.3 and 1.4
at least, targeting at 2.0 is likely best.

Fixes: #7463
Change-Id: Ie4bae767747bba0d03fb13eaff579d4bab0b1bc2

5 files changed, 115 insertions, 35 deletions

diff --git a/doc/build/changelog/unreleased_20/7463.rst b/doc/build/changelog/unreleased_20/7463.rst
new file mode 100644
index 000000000..766dd530a
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/7463.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 7463
+
+    Fixed performance regression which appeared at least in version 1.3 if not
+    earlier (sometime after 1.0) where the loading of deferred columns, those
+    explicitly mapped with :func:`_orm.defer` as opposed to non-deferred
+    columns that were expired, from a joined inheritance subclass would not use
+    the "optimized" query which only queried the immediate table that contains
+    the unloaded columns, instead running a full ORM query which would emit a
+    JOIN for all base tables, which is not necessary when only loading columns
+    from the subclass.
+
diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index d6ee9b7a7..a37d83cfe 100644
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -16,7 +16,6 @@ as well as some of the attribute loading strategies.
 from . import attributes
 from . import exc as orm_exc
 from . import path_registry
-from . import strategy_options
 from .base import _DEFER_FOR_STATE
 from .base import _RAISE_FOR_STATE
 from .base import _SET_DEFERRED_EXPIRED
@@ -1386,25 +1385,14 @@ def load_scalar_attributes(mapper, state, attribute_names, passive):
         attribute_names = attribute_names.intersection(mapper.attrs.keys())
 
     if mapper.inherits and not mapper.concrete:
-        # because we are using Core to produce a select() that we
-        # pass to the Query, we aren't calling setup() for mapped
-        # attributes; in 1.0 this means deferred attrs won't get loaded
-        # by default
         statement = mapper._optimized_get_statement(state, attribute_names)
         if statement is not None:
-            # this was previously aliased(mapper, statement), however,
-            # statement is a select() and Query's coercion now raises for this
-            # since you can't "select" from a "SELECT" statement.  only
-            # from_statement() allows this.
-            # note: using from_statement() here means there is an adaption
-            # with adapt_on_names set up.  the other option is to make the
-            # aliased() against a subquery which affects the SQL.
 
             from .query import FromStatement
 
-            stmt = FromStatement(mapper, statement).options(
-                strategy_options.Load(mapper).undefer("*")
-            )
+            # undefer() isn't needed here because statement has the
+            # columns needed already, this implicitly undefers that column
+            stmt = FromStatement(mapper, statement)
 
             result = load_on_ident(
                 session,
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index ff4ac9d33..51e81e104 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -24,6 +24,7 @@ from . import util as orm_util
 from .base import _DEFER_FOR_STATE
 from .base import _RAISE_FOR_STATE
 from .base import _SET_DEFERRED_EXPIRED
+from .base import PASSIVE_OFF
 from .context import _column_descriptions
 from .context import ORMCompileState
 from .context import QueryContext
@@ -512,19 +513,9 @@ class DeferredColumnLoader(LoaderStrategy):
         if self.raiseload:
             self._invoke_raise_load(state, passive, "raise")
 
-        if (
-            loading.load_on_ident(
-                session,
-                sql.select(localparent).set_label_style(
-                    LABEL_STYLE_TABLENAME_PLUS_COL
-                ),
-                state.key,
-                only_load_props=group,
-                refresh_state=state,
-            )
-            is None
-        ):
-            raise orm_exc.ObjectDeletedError(state)
+        loading.load_scalar_attributes(
+            state.mapper, state, set(group), PASSIVE_OFF
+        )
 
         return attributes.ATTR_WAS_SET
 
diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py
index 339a6b956..1dc01053b 100644
--- a/test/orm/inheritance/test_basic.py
+++ b/test/orm/inheritance/test_basic.py
@@ -1777,7 +1777,19 @@ class PassiveDeletesTest(fixtures.MappedTest):
 
 
 class OptimizedGetOnDeferredTest(fixtures.MappedTest):
-    """test that the 'optimized get' path accommodates deferred columns."""
+    """test that the 'optimized get' path accommodates deferred columns.
+
+    Original issue tested is #3468, where loading of a deferred column
+    in an inherited subclass would fail.
+
+    At some point, the logic tested was no longer used and a less efficient
+    query was used to load these columns, but the test here did not inspect
+    the SQL such that this would be detected.
+
+    Test was then revised to more carefully test and now targets
+    #7463 as well.
+
+    """
 
     @classmethod
     def define_tables(cls, metadata):
@@ -1787,6 +1799,7 @@ class OptimizedGetOnDeferredTest(fixtures.MappedTest):
             Column(
                 "id", Integer, primary_key=True, test_needs_autoincrement=True
             ),
+            Column("type", String(10)),
         )
         Table(
             "b",
@@ -1808,11 +1821,12 @@ class OptimizedGetOnDeferredTest(fixtures.MappedTest):
         A, B = cls.classes("A", "B")
         a, b = cls.tables("a", "b")
 
-        cls.mapper_registry.map_imperatively(A, a)
+        cls.mapper_registry.map_imperatively(A, a, polymorphic_on=a.c.type)
         cls.mapper_registry.map_imperatively(
             B,
             b,
             inherits=A,
+            polymorphic_identity="b",
             properties={
                 "data": deferred(b.c.data),
                 "expr": column_property(b.c.data + "q", deferred=True),
@@ -1825,8 +1839,17 @@ class OptimizedGetOnDeferredTest(fixtures.MappedTest):
         b1 = B(data="x")
         sess.add(b1)
         sess.flush()
+        b_id = b1.id
 
-        eq_(b1.expr, "xq")
+        with self.sql_execution_asserter(testing.db) as asserter:
+            eq_(b1.expr, "xq")
+        asserter.assert_(
+            CompiledSQL(
+                "SELECT b.data || :data_1 AS anon_1 "
+                "FROM b WHERE :param_1 = b.id",
+                [{"param_1": b_id, "data_1": "q"}],
+            )
+        )
 
     def test_expired_column(self):
         A, B = self.classes("A", "B")
@@ -1834,9 +1857,74 @@ class OptimizedGetOnDeferredTest(fixtures.MappedTest):
         b1 = B(data="x")
         sess.add(b1)
         sess.flush()
+        b_id = b1.id
         sess.expire(b1, ["data"])
 
+        with self.sql_execution_asserter(testing.db) as asserter:
+            eq_(b1.data, "x")
+        # uses efficient statement w/o JOIN to a
+        asserter.assert_(
+            CompiledSQL(
+                "SELECT b.data AS b_data FROM b WHERE :param_1 = b.id",
+                [{"param_1": b_id}],
+            )
+        )
+
+    def test_load_from_unloaded_subclass(self):
+        A, B = self.classes("A", "B")
+        sess = fixture_session()
+        b1 = B(data="x")
+        sess.add(b1)
+        sess.commit()
+        b_id = b1.id
+        sess.close()
+
+        # load polymorphically in terms of A, so that B needs another
+        # SELECT
+        b1 = sess.execute(select(A)).scalar()
+
+        # it's not loaded
+        assert "data" not in b1.__dict__
+
+        # but it loads successfully when requested
+        with self.sql_execution_asserter(testing.db) as asserter:
+            eq_(b1.data, "x")
+
+        # uses efficient statement w/o JOIN to a
+        asserter.assert_(
+            CompiledSQL(
+                "SELECT b.data AS b_data FROM b WHERE :param_1 = b.id",
+                [{"param_1": b_id}],
+            )
+        )
+
+    def test_load_from_expired_subclass(self):
+        A, B = self.classes("A", "B")
+        sess = fixture_session()
+        b1 = B(data="x")
+        sess.add(b1)
+        sess.commit()
+        b_id = b1.id
+        sess.close()
+
+        b1 = sess.execute(select(A)).scalar()
+
+        # it's not loaded
+        assert "data" not in b1.__dict__
+
         eq_(b1.data, "x")
+        sess.expire(b1, ["data"])
+
+        with self.sql_execution_asserter(testing.db) as asserter:
+            eq_(b1.data, "x")
+
+        # uses efficient statement w/o JOIN to a
+        asserter.assert_(
+            CompiledSQL(
+                "SELECT b.data AS b_data FROM b WHERE :param_1 = b.id",
+                [{"param_1": b_id}],
+            )
+        )
 
 
 class JoinedNoFKSortingTest(fixtures.MappedTest):
diff --git a/test/orm/inheritance/test_poly_loading.py b/test/orm/inheritance/test_poly_loading.py
index fcaf470b4..0a0338831 100644
--- a/test/orm/inheritance/test_poly_loading.py
+++ b/test/orm/inheritance/test_poly_loading.py
@@ -846,10 +846,10 @@ class IgnoreOptionsOnSubclassAttrLoad(fixtures.DeclarativeMappedTest):
             # call to the deferred load put a deferred loader on the attribute
             expected.append(
                 CompiledSQL(
-                    "SELECT sub_entity.name AS sub_entity_name FROM entity "
-                    "JOIN sub_entity ON entity.id = sub_entity.id "
-                    "WHERE entity.id = :pk_1",
-                    [{"pk_1": entity_id}],
+                    "SELECT sub_entity.name AS sub_entity_name "
+                    "FROM sub_entity "
+                    "WHERE :param_1 = sub_entity.id",
+                    [{"param_1": entity_id}],
                 )
             )