]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
restore graceful degrade of subqueryload w from_statement
authorMike Bayer <mike_mp@zzzcomputing.com>
Sun, 26 Dec 2021 16:25:00 +0000 (11:25 -0500)
committerMike Bayer <mike_mp@zzzcomputing.com>
Sun, 26 Dec 2021 19:45:45 +0000 (14:45 -0500)
Fixed regression from 1.3 where the "subqueryload" loader strategy would
fail with a stack trace if used against a query that made use of
:meth:`_orm.Query.from_statement` or :meth:`_sql.Select.from_statement`. As
subqueryload requires modifying the original statement, it's not compatible
with the "from_statement" use case, especially for statements made against
the :func:`_sql.text` construct. The behavior now is equivalent to that of
1.3 and previously, which is that the loader strategy silently degrades to
not be used for such statements, typically falling back to using the
lazyload strategy.

Fixes: #7505
Change-Id: I950800dc86a77f8320a5e696edce1ff2c84b1eb9

doc/build/changelog/unreleased_14/7505.rst [new file with mode: 0644]
lib/sqlalchemy/orm/context.py
lib/sqlalchemy/orm/strategies.py
test/orm/test_eager_relations.py
test/orm/test_query.py
test/orm/test_selectin_relations.py
test/orm/test_subquery_relations.py

diff --git a/doc/build/changelog/unreleased_14/7505.rst b/doc/build/changelog/unreleased_14/7505.rst
new file mode 100644 (file)
index 0000000..b017c0a
--- /dev/null
@@ -0,0 +1,14 @@
+.. change::
+    :tags: bug, orm, regression
+    :tickets: 7505
+
+    Fixed regression from 1.3 where the "subqueryload" loader strategy would
+    fail with a stack trace if used against a query that made use of
+    :meth:`_orm.Query.from_statement` or :meth:`_sql.Select.from_statement`. As
+    subqueryload requires modifying the original statement, it's not compatible
+    with the "from_statement" use case, especially for statements made against
+    the :func:`_sql.text` construct. The behavior now is equivalent to that of
+    1.3 and previously, which is that the loader strategy silently degrades to
+    not be used for such statements, typically falling back to using the
+    lazyload strategy.
+
index 4e2586203c2f9a834e7cdfa65b3cd9cd3e6db10e..e834b22e8518259f9f557f8b862aa3adf1623f83 100644 (file)
@@ -382,6 +382,12 @@ class ORMCompileState(CompileState):
             for m in m2.iterate_to_root():  # TODO: redundant ?
                 self._polymorphic_adapters[m.local_table] = adapter
 
+    @classmethod
+    def _create_entities_collection(cls, query, legacy):
+        raise NotImplementedError(
+            "this method only works for ORMSelectCompileState"
+        )
+
 
 @sql.base.CompileState.plugin_for("orm", "orm_from_statement")
 class ORMFromStatementCompileState(ORMCompileState):
index 51e81e104d7c393977b5f5b5703400aca7293326..d5f5f8527d34879f1e0132e4ab758650d7708c6a 100644 (file)
@@ -27,6 +27,7 @@ from .base import _SET_DEFERRED_EXPIRED
 from .base import PASSIVE_OFF
 from .context import _column_descriptions
 from .context import ORMCompileState
+from .context import ORMSelectCompileState
 from .context import QueryContext
 from .interfaces import LoaderStrategy
 from .interfaces import StrategizedProperty
@@ -1791,6 +1792,11 @@ class SubqueryLoader(PostLoader):
         # the other post loaders, however we have this here for consistency
         elif self._check_recursive_postload(context, path, self.join_depth):
             return
+        elif not isinstance(context.compile_state, ORMSelectCompileState):
+            # issue 7505 - subqueryload() in 1.3 and previous would silently
+            # degrade for from_statement() without warning. this behavior
+            # is restored here
+            return
 
         if not self.parent.class_manager[self.key].impl.supports_population:
             raise sa_exc.InvalidRequestError(
index 4bd0f14171de70e30e15475fb16029d8e4d99328..6d6d10deafdc3841d165cf71685d97ab11cb81db 100644 (file)
@@ -86,6 +86,99 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
         )
         eq_(self.static.user_address_result, q.order_by(User.id).all())
 
+    @testing.combinations(True, False)
+    def test_from_statement(self, legacy):
+        users, Address, addresses, User = (
+            self.tables.users,
+            self.classes.Address,
+            self.tables.addresses,
+            self.classes.User,
+        )
+
+        self.mapper_registry.map_imperatively(
+            User,
+            users,
+            properties={
+                "addresses": relationship(
+                    self.mapper_registry.map_imperatively(Address, addresses),
+                    order_by=Address.id,
+                )
+            },
+        )
+
+        sess = fixture_session()
+
+        stmt = select(User).where(User.id == 7)
+
+        def go():
+            if legacy:
+                ret = (
+                    sess.query(User)
+                    .from_statement(stmt)
+                    .options(joinedload(User.addresses))
+                    .all()
+                )
+            else:
+                ret = sess.scalars(
+                    select(User)
+                    .from_statement(stmt)
+                    .options(joinedload(User.addresses))
+                ).all()
+
+            eq_(self.static.user_address_result[0:1], ret)
+
+        # joinedload can't be applied here so this necessarily
+        # has to lazy load the addresses
+        self.assert_sql_count(testing.db, go, 2)
+
+    @testing.combinations(True, False)
+    def test_from_statement_contains_eager(self, legacy):
+        users, Address, addresses, User = (
+            self.tables.users,
+            self.classes.Address,
+            self.tables.addresses,
+            self.classes.User,
+        )
+
+        self.mapper_registry.map_imperatively(
+            User,
+            users,
+            properties={
+                "addresses": relationship(
+                    self.mapper_registry.map_imperatively(Address, addresses),
+                    order_by=Address.id,
+                )
+            },
+        )
+
+        sess = fixture_session()
+
+        # for contains_eager, Address.id is enough for it to be picked up
+        stmt = (
+            select(User, Address.id).where(User.id == 7).join(User.addresses)
+        )
+
+        def go():
+            if legacy:
+                ret = (
+                    sess.query(User)
+                    .from_statement(stmt)
+                    .options(contains_eager(User.addresses))
+                    .all()
+                )
+            else:
+                ret = sess.scalars(
+                    select(User)
+                    .from_statement(stmt)
+                    .options(contains_eager(User.addresses))
+                ).all()
+
+            eq_(self.static.user_address_result[0:1], ret)
+
+        # joinedload can't be applied here so this necessarily
+        # has to lazy load the addresses
+        self.assert_sql_count(testing.db, go, 1)
+
     def test_no_render_in_subquery(self):
         """test #6378"""
 
index d3ca9521f82366a1428f212de7779dd79e586455..74def1c2b826481ff4bd663e86e681fb6cac07c5 100644 (file)
@@ -5975,12 +5975,11 @@ class TextTest(QueryTest, AssertsCompiledSQL):
         (
             False,
             subqueryload,
-            # sqlite seems happy to interpret the broken SQL and give you the
-            # correct result somehow, this is a bug in SQLite so don't rely
-            # upon it doing that
-            testing.fails("not working yet") + testing.skip_if("sqlite"),
         ),
-        (True, subqueryload, testing.fails("not sure about implementation")),
+        (
+            True,
+            subqueryload,
+        ),
         (False, selectinload),
         (True, selectinload),
     )
@@ -6008,7 +6007,12 @@ class TextTest(QueryTest, AssertsCompiledSQL):
         def go():
             eq_(set(q.all()), set(self.static.user_address_result))
 
-        self.assert_sql_count(testing.db, go, 2)
+        if loader_option is subqueryload:
+            # subqueryload necessarily degrades to lazy loads for a text
+            # statement.
+            self.assert_sql_count(testing.db, go, 5)
+        else:
+            self.assert_sql_count(testing.db, go, 2)
 
     def test_whereclause(self):
         User = self.classes.User
index 2add1015ffc166fa2dd73e1500f5b1e8217cf93c..7a5bb0e7edbc9921ccb659bcfcac9c02643810bb 100644 (file)
@@ -89,6 +89,48 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
 
         self.assert_sql_count(testing.db, go, 2)
 
+    @testing.combinations(True, False)
+    def test_from_statement(self, legacy):
+        users, Address, addresses, User = (
+            self.tables.users,
+            self.classes.Address,
+            self.tables.addresses,
+            self.classes.User,
+        )
+
+        self.mapper_registry.map_imperatively(
+            User,
+            users,
+            properties={
+                "addresses": relationship(
+                    self.mapper_registry.map_imperatively(Address, addresses),
+                    order_by=Address.id,
+                )
+            },
+        )
+        sess = fixture_session()
+
+        stmt = select(User).where(User.id == 7)
+
+        def go():
+            if legacy:
+                ret = (
+                    sess.query(User)
+                    .from_statement(stmt)
+                    .options(selectinload(User.addresses))
+                    .all()
+                )
+            else:
+                ret = sess.scalars(
+                    select(User)
+                    .from_statement(stmt)
+                    .options(selectinload(User.addresses))
+                ).all()
+
+            eq_(self.static.user_address_result[0:1], ret)
+
+        self.assert_sql_count(testing.db, go, 2)
+
     def user_dingaling_fixture(self):
         users, Dingaling, User, dingalings, Address, addresses = (
             self.tables.users,
index e07da638f2c623b824be15faaeb78f259f35f202..c48fe458815f31993e0a54de38bb1529a3a434b3 100644 (file)
@@ -28,6 +28,7 @@ from sqlalchemy.testing import is_
 from sqlalchemy.testing import is_not
 from sqlalchemy.testing import is_true
 from sqlalchemy.testing.assertsql import CompiledSQL
+from sqlalchemy.testing.assertsql import Or
 from sqlalchemy.testing.entities import ComparableEntity
 from sqlalchemy.testing.fixtures import fixture_session
 from sqlalchemy.testing.schema import Column
@@ -89,6 +90,71 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
 
         self.assert_sql_count(testing.db, go, 2)
 
+    @testing.combinations(True, False)
+    def test_from_statement(self, legacy):
+        users, Address, addresses, User = (
+            self.tables.users,
+            self.classes.Address,
+            self.tables.addresses,
+            self.classes.User,
+        )
+
+        self.mapper_registry.map_imperatively(
+            User,
+            users,
+            properties={
+                "addresses": relationship(
+                    self.mapper_registry.map_imperatively(Address, addresses),
+                    order_by=Address.id,
+                )
+            },
+        )
+        sess = fixture_session()
+
+        stmt = select(User).where(User.id == 7)
+
+        with self.sql_execution_asserter(testing.db) as asserter:
+            if legacy:
+                ret = (
+                    sess.query(User)
+                    # .where(User.id == 7)
+                    .from_statement(stmt)
+                    .options(subqueryload(User.addresses))
+                    .all()
+                )
+            else:
+                ret = sess.scalars(
+                    select(User)
+                    .from_statement(stmt)
+                    .options(subqueryload(User.addresses))
+                ).all()
+
+            eq_(self.static.user_address_result[0:1], ret)
+
+        asserter.assert_(
+            Or(
+                CompiledSQL(
+                    "SELECT users.id AS users_id, users.name AS users_name "
+                    "FROM users WHERE users.id = :id_1",
+                    [{"id_1": 7}],
+                ),
+                CompiledSQL(
+                    "SELECT users.id, users.name "
+                    "FROM users WHERE users.id = :id_1",
+                    [{"id_1": 7}],
+                ),
+            ),
+            # issue 7505
+            # subqueryload degrades for a from_statement.  this is a lazyload
+            CompiledSQL(
+                "SELECT addresses.id AS addresses_id, addresses.user_id AS "
+                "addresses_user_id, addresses.email_address AS "
+                "addresses_email_address FROM addresses "
+                "WHERE :param_1 = addresses.user_id ORDER BY addresses.id",
+                [{"param_1": 7}],
+            ),
+        )
+
     def test_params_arent_cached(self):
         users, Address, addresses, User = (
             self.tables.users,