]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Clone internals for Select._correlate_except collection as well as _correlate
authorMike Bayer <mike_mp@zzzcomputing.com>
Mon, 11 Mar 2019 03:34:33 +0000 (23:34 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Mon, 11 Mar 2019 03:44:08 +0000 (23:44 -0400)
Fixed bug where use of :func:`.with_polymorphic` or other aliased construct
would not properly adapt when the aliased target were used as the
:meth:`.Select.correlate_except` target of a subquery used inside of a
:func:`.column_property`.  This required a fix to the clause adaption
mechanics to properly handle a selectable that shows up in the "correlate
except" list, in a similar manner as which occurs for selectables that show
up in the "correlate" list.  This is ultimately a fairly fundamental bug
that has lasted for a long time but it is hard to come across it.

Fixes: #4537
Change-Id: Ibb97d4eea18b3c452aad519dd14919bfb84d422f

doc/build/changelog/unreleased_13/4537.rst [new file with mode: 0644]
lib/sqlalchemy/sql/selectable.py
test/orm/inheritance/test_assorted_poly.py
test/sql/test_generative.py

diff --git a/doc/build/changelog/unreleased_13/4537.rst b/doc/build/changelog/unreleased_13/4537.rst
new file mode 100644 (file)
index 0000000..62b1894
--- /dev/null
@@ -0,0 +1,13 @@
+.. change::
+   :tags: bug, orm
+   :tickets: 4537
+
+   Fixed bug where use of :func:`.with_polymorphic` or other aliased construct
+   would not properly adapt when the aliased target were used as the
+   :meth:`.Select.correlate_except` target of a subquery used inside of a
+   :func:`.column_property`. This required a fix to the clause adaption
+   mechanics to properly handle a selectable that shows up in the "correlate
+   except" list, in a similar manner as which occurs for selectables that show
+   up in the "correlate" list.  This is ultimately a fairly fundamental bug
+   that has lasted for a long time but it is hard to come across it.
+
index cc6d2bcc518edf4d5675da40557b89b98d25fa3a..d4528f0c317c85e1911d344f26d7cc7fb2d43e47 100644 (file)
@@ -3227,6 +3227,14 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect):
         self._correlate = set(clone(f) for f in self._correlate).union(
             self._correlate
         )
+
+        # do something similar for _correlate_except - this is a more
+        # unusual case but same idea applies
+        if self._correlate_except:
+            self._correlate_except = set(
+                clone(f) for f in self._correlate_except
+            ).union(self._correlate_except)
+
         # 4. clone other things.   The difficulty here is that Column
         # objects are not actually cloned, and refer to their original
         # .table, resulting in the wrong "from" parent after a clone
index ef09211a73e6f10f0b9fb0c8a19dff77ad0e158c..525824669c13c05ada80e527f3624afef593225a 100644 (file)
@@ -16,6 +16,8 @@ from sqlalchemy import Unicode
 from sqlalchemy import util
 from sqlalchemy.orm import class_mapper
 from sqlalchemy.orm import clear_mappers
+from sqlalchemy.orm import column_property
+from sqlalchemy.orm import contains_eager
 from sqlalchemy.orm import create_session
 from sqlalchemy.orm import join
 from sqlalchemy.orm import joinedload
@@ -24,6 +26,7 @@ from sqlalchemy.orm import polymorphic_union
 from sqlalchemy.orm import Query
 from sqlalchemy.orm import relationship
 from sqlalchemy.orm import Session
+from sqlalchemy.orm import with_polymorphic
 from sqlalchemy.orm.interfaces import MANYTOONE
 from sqlalchemy.testing import AssertsExecutionResults
 from sqlalchemy.testing import eq_
@@ -503,7 +506,7 @@ for jointype in ["join1", "join2", "join3", "join4"]:
     for data in (True, False):
         _fn = _generate_test(jointype, data)
         setattr(RelationshipTest3, _fn.__name__, _fn)
-del func
+del _fn
 
 
 class RelationshipTest4(fixtures.MappedTest):
@@ -2139,3 +2142,111 @@ class ColSubclassTest(
             "(a AS a_1 JOIN b AS b_1 ON a_1.id = b_1.id) "
             "ON a.id = b_1.id WHERE b_1.x = :x_1",
         )
+
+
+class CorrelateExceptWPolyAdaptTest(
+    fixtures.DeclarativeMappedTest, testing.AssertsCompiledSQL
+):
+    # test [ticket:4537]'s test case.
+
+    run_create_tables = run_deletes = None
+    run_setup_classes = run_setup_mappers = run_define_tables = "each"
+    __dialect__ = "default"
+
+    def _fixture(self, use_correlate_except):
+
+        Base = self.DeclarativeBasic
+
+        class Superclass(Base):
+            __tablename__ = "s1"
+            id = Column(Integer, primary_key=True)
+            common_id = Column(ForeignKey("c.id"))
+            common_relationship = relationship(
+                "Common", uselist=False, innerjoin=True, lazy="noload"
+            )
+            discriminator_field = Column(String)
+            __mapper_args__ = {
+                "polymorphic_identity": "superclass",
+                "polymorphic_on": discriminator_field,
+            }
+
+        class Subclass(Superclass):
+            __tablename__ = "s2"
+            id = Column(ForeignKey("s1.id"), primary_key=True)
+            __mapper_args__ = {"polymorphic_identity": "subclass"}
+
+        class Common(Base):
+            __tablename__ = "c"
+            id = Column(Integer, primary_key=True)
+
+            if use_correlate_except:
+                num_superclass = column_property(
+                    select([func.count(Superclass.id)])
+                    .where(Superclass.common_id == id)
+                    .correlate_except(Superclass)
+                )
+
+        if not use_correlate_except:
+            Common.num_superclass = column_property(
+                select([func.count(Superclass.id)])
+                .where(Superclass.common_id == Common.id)
+                .correlate(Common)
+            )
+
+        return Common, Superclass
+
+    def test_poly_query_on_correlate(self):
+        Common, Superclass = self._fixture(False)
+
+        poly = with_polymorphic(Superclass, "*")
+
+        s = Session()
+        q = (
+            s.query(poly)
+            .options(contains_eager(poly.common_relationship))
+            .join(poly.common_relationship)
+            .filter(Common.id == 1)
+        )
+
+        # note the order of c.id, subquery changes based on if we
+        # used correlate or correlate_except; this is only with the
+        # patch in place.   Not sure why this happens.
+        self.assert_compile(
+            q,
+            "SELECT c.id AS c_id, (SELECT count(s1.id) AS count_1 "
+            "FROM s1 LEFT OUTER JOIN s2 ON s1.id = s2.id "
+            "WHERE s1.common_id = c.id) AS anon_1, "
+            "s1.id AS s1_id, "
+            "s1.common_id AS s1_common_id, "
+            "s1.discriminator_field AS s1_discriminator_field, "
+            "s2.id AS s2_id FROM s1 "
+            "LEFT OUTER JOIN s2 ON s1.id = s2.id "
+            "JOIN c ON c.id = s1.common_id WHERE c.id = :id_1",
+        )
+
+    def test_poly_query_on_correlate_except(self):
+        Common, Superclass = self._fixture(True)
+
+        poly = with_polymorphic(Superclass, "*")
+
+        s = Session()
+        q = (
+            s.query(poly)
+            .options(contains_eager(poly.common_relationship))
+            .join(poly.common_relationship)
+            .filter(Common.id == 1)
+        )
+
+        # c.id, subquery are reversed.
+        self.assert_compile(
+            q,
+            "SELECT (SELECT count(s1.id) AS count_1 "
+            "FROM s1 LEFT OUTER JOIN s2 ON s1.id = s2.id "
+            "WHERE s1.common_id = c.id) AS anon_1, "
+            "c.id AS c_id, s1.id AS s1_id, "
+            "s1.common_id AS s1_common_id, "
+            "s1.discriminator_field AS s1_discriminator_field, "
+            "s2.id AS s2_id FROM s1 "
+            "LEFT OUTER JOIN s2 ON s1.id = s2.id "
+            "JOIN c ON c.id = s1.common_id WHERE c.id = :id_1",
+        )
index ccb5b15ead8df8cdbce08342d688c74884ca1d49..9b902e8ffd7c8801265c081b7076fe3b1e32f819 100644 (file)
@@ -1088,6 +1088,90 @@ class ClauseAdapterTest(fixtures.TestBase, AssertsCompiledSQL):
             "AS anon_1 FROM table1 AS t1alias",
         )
 
+    def test_correlate_except_on_clone(self):
+        # test [ticket:4537]'s issue
+
+        t1alias = t1.alias("t1alias")
+        j = t1.join(t1alias, t1.c.col1 == t1alias.c.col2)
+
+        vis = sql_util.ClauseAdapter(j)
+
+        # "control" subquery - uses correlate which has worked w/ adaption
+        # for a long time
+        control_s = (
+            select([t2.c.col1])
+            .where(t2.c.col1 == t1.c.col1)
+            .correlate(t2)
+            .as_scalar()
+        )
+
+        # test subquery - given only t1 and t2 in the enclosing selectable,
+        # will do the same thing as the "control" query since the correlation
+        # works out the same
+        s = (
+            select([t2.c.col1])
+            .where(t2.c.col1 == t1.c.col1)
+            .correlate_except(t1)
+            .as_scalar()
+        )
+
+        # use both subqueries in statements
+        control_stmt = select([control_s, t1.c.col1, t2.c.col1]).select_from(
+            t1.join(t2, t1.c.col1 == t2.c.col1)
+        )
+
+        stmt = select([s, t1.c.col1, t2.c.col1]).select_from(
+            t1.join(t2, t1.c.col1 == t2.c.col1)
+        )
+        # they are the same
+        self.assert_compile(
+            control_stmt,
+            "SELECT "
+            "(SELECT table2.col1 FROM table1 "
+            "WHERE table2.col1 = table1.col1) AS anon_1, "
+            "table1.col1, table2.col1 "
+            "FROM table1 "
+            "JOIN table2 ON table1.col1 = table2.col1",
+        )
+        self.assert_compile(
+            stmt,
+            "SELECT "
+            "(SELECT table2.col1 FROM table1 "
+            "WHERE table2.col1 = table1.col1) AS anon_1, "
+            "table1.col1, table2.col1 "
+            "FROM table1 "
+            "JOIN table2 ON table1.col1 = table2.col1",
+        )
+
+        # now test against the adaption of "t1" into "t1 JOIN t1alias".
+        # note in the control case, we aren't actually testing that
+        # Select is processing the "correlate" list during the adaption
+        # since we aren't adapting the "correlate"
+        self.assert_compile(
+            vis.traverse(control_stmt),
+            "SELECT "
+            "(SELECT table2.col1 FROM "
+            "table1 JOIN table1 AS t1alias ON table1.col1 = t1alias.col2 "
+            "WHERE table2.col1 = table1.col1) AS anon_1, "
+            "table1.col1, table2.col1 "
+            "FROM table1 JOIN table1 AS t1alias ON table1.col1 = t1alias.col2 "
+            "JOIN table2 ON table1.col1 = table2.col1",
+        )
+
+        # but here, correlate_except() does have the thing we're adapting
+        # so whatever is in there has to be expanded out to include
+        # the adaptation target, in this case "t1 JOIN t1alias".
+        self.assert_compile(
+            vis.traverse(stmt),
+            "SELECT "
+            "(SELECT table2.col1 FROM "
+            "table1 JOIN table1 AS t1alias ON table1.col1 = t1alias.col2 "
+            "WHERE table2.col1 = table1.col1) AS anon_1, "
+            "table1.col1, table2.col1 "
+            "FROM table1 JOIN table1 AS t1alias ON table1.col1 = t1alias.col2 "
+            "JOIN table2 ON table1.col1 = table2.col1",
+        )
+
     @testing.fails_on_everything_except()
     def test_joins_dont_adapt(self):
         # adapting to a join, i.e. ClauseAdapter(t1.join(t2)), doesn't