fix linter JOIN logic; fix PostgreSQL ARRAY op comparison

Adjusted the "from linter" warning feature to accommodate for a chain of
joins more than one level deep where the ON clauses don't explicitly match
up the targets, such as an expression such as "ON TRUE". This mode of use
is intended to cancel the cartesian product warning simply by the fact that
there's a JOIN from "a to b", which was not working for the case where the
chain of joins had more than one element.

this incurs a bit more compiler overhead that comes out in profiling
but is not extensive.

Added the "is_comparison" flag to the PostgreSQL "overlaps",
"contained_by", "contains" operators, so that they work in relevant ORM
contexts as well as in conjunction with the "from linter" feature.

Fixes: #6886
Change-Id: I078dc3fe6d4f7871ffe4ebac3e71e62f3f213d12

diff --git a/doc/build/changelog/unreleased_14/6886.rst b/doc/build/changelog/unreleased_14/6886.rst
new file mode 100644 (file)
index 0000000..b489991
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/6886.rst
@@ -0,0 +1,18 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 6886
+
+    Adjusted the "from linter" warning feature to accommodate for a chain of
+    joins more than one level deep where the ON clauses don't explicitly match
+    up the targets, such as an expression such as "ON TRUE". This mode of use
+    is intended to cancel the cartesian product warning simply by the fact that
+    there's a JOIN from "a to b", which was not working for the case where the
+    chain of joins had more than one element.
+
+.. change::
+    :tags: bug, postgresql
+    :tickets: 6886
+
+    Added the "is_comparison" flag to the PostgreSQL "overlaps",
+    "contained_by", "contains" operators, so that they work in relevant ORM
+    contexts as well as in conjunction with the "from linter" feature.

diff --git a/lib/sqlalchemy/dialects/postgresql/array.py b/lib/sqlalchemy/dialects/postgresql/array.py
index d8e54ae0c415e5170a080509e88245c81dc2f2ed..1b036ac32326504cdd5c1d861220b3e2c2b4c10c 100644 (file)
--- a/lib/sqlalchemy/dialects/postgresql/array.py
+++ b/lib/sqlalchemy/dialects/postgresql/array.py
@@ -153,11 +153,11 @@ class array(expression.ClauseList, expression.ColumnElement):
             return self
 
 
-CONTAINS = operators.custom_op("@>", precedence=5)
+CONTAINS = operators.custom_op("@>", precedence=5, is_comparison=True)
 
-CONTAINED_BY = operators.custom_op("<@", precedence=5)
+CONTAINED_BY = operators.custom_op("<@", precedence=5, is_comparison=True)
 
-OVERLAP = operators.custom_op("&&", precedence=5)
+OVERLAP = operators.custom_op("&&", precedence=5, is_comparison=True)
 
 
 class ARRAY(sqltypes.ARRAY):

diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index 861b0be1641fff4b57d7fd4db14f9b0f86d187d0..94a0b42017d19061fbafdd98765580133981322a 100644 (file)
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -3559,7 +3559,11 @@ class SQLCompiler(Compiled):
 
     def visit_join(self, join, asfrom=False, from_linter=None, **kwargs):
         if from_linter:
-            from_linter.edges.add((join.left, join.right))
+            from_linter.edges.update(
+                itertools.product(
+                    join.left._from_objects, join.right._from_objects
+                )
+            )
 
         if join.full:
             join_type = " FULL OUTER JOIN "

diff --git a/test/dialect/postgresql/test_compiler.py b/test/dialect/postgresql/test_compiler.py
index d2c81e63f8da02c825b1913f01fab53922746e5a..0b56d898766cae6c288f93e904d909470a381e8c 100644 (file)
--- a/test/dialect/postgresql/test_compiler.py
+++ b/test/dialect/postgresql/test_compiler.py
@@ -1508,6 +1508,39 @@ class CompileTest(fixtures.TestBase, AssertsCompiledSQL):
             checkparams={"param_1": 7},
         )
 
+    @testing.combinations(
+        (lambda c: c.overlap, "&&"),
+        (lambda c: c.contains, "@>"),
+        (lambda c: c.contained_by, "<@"),
+    )
+    def test_overlap_no_cartesian(self, op_fn, expected_op):
+        """test #6886"""
+        t1 = table(
+            "t1",
+            column("id", Integer),
+            column("ancestor_ids", postgresql.ARRAY(Integer)),
+        )
+
+        t1a = t1.alias()
+        t1b = t1.alias()
+
+        stmt = (
+            select(t1, t1a, t1b)
+            .where(op_fn(t1a.c.ancestor_ids)(postgresql.array((t1.c.id,))))
+            .where(op_fn(t1b.c.ancestor_ids)(postgresql.array((t1.c.id,))))
+        )
+
+        self.assert_compile(
+            stmt,
+            "SELECT t1.id, t1.ancestor_ids, t1_1.id AS id_1, "
+            "t1_1.ancestor_ids AS ancestor_ids_1, t1_2.id AS id_2, "
+            "t1_2.ancestor_ids AS ancestor_ids_2 "
+            "FROM t1, t1 AS t1_1, t1 AS t1_2 "
+            "WHERE t1_1.ancestor_ids %(op)s ARRAY[t1.id] "
+            "AND t1_2.ancestor_ids %(op)s ARRAY[t1.id]" % {"op": expected_op},
+            from_linting=True,
+        )
+
     @testing.combinations((True,), (False,))
     def test_array_zero_indexes(self, zero_indexes):
         c = Column("x", postgresql.ARRAY(Integer, zero_indexes=zero_indexes))

diff --git a/test/profiles.txt b/test/profiles.txt
index be049f5eb1d8cf9934af46be511dc7f8c45badb2..d3b2c209543f3aa325200d97d17022d81cbd4426 100644 (file)
--- a/test/profiles.txt
+++ b/test/profiles.txt
@@ -291,10 +291,10 @@ test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_build_query x86_64_linux_cp
 
 # TEST: test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results
 
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results x86_64_linux_cpython_2.7_sqlite_pysqlite_dbapiunicode_cextensions 400805
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results x86_64_linux_cpython_2.7_sqlite_pysqlite_dbapiunicode_nocextensions 417205
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results x86_64_linux_cpython_3.9_sqlite_pysqlite_dbapiunicode_cextensions 405405
-test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results x86_64_linux_cpython_3.9_sqlite_pysqlite_dbapiunicode_nocextensions 423905
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results x86_64_linux_cpython_2.7_sqlite_pysqlite_dbapiunicode_cextensions 420805
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results x86_64_linux_cpython_2.7_sqlite_pysqlite_dbapiunicode_nocextensions 437205
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results x86_64_linux_cpython_3.9_sqlite_pysqlite_dbapiunicode_cextensions 425405
+test.aaa_profiling.test_orm.JoinedEagerLoadTest.test_fetch_results x86_64_linux_cpython_3.9_sqlite_pysqlite_dbapiunicode_nocextensions 443905
 
 # TEST: test.aaa_profiling.test_orm.LoadManyToOneFromIdentityTest.test_many_to_one_load_identity
 

diff --git a/test/sql/test_from_linter.py b/test/sql/test_from_linter.py
index 9e0ededecc3e5bae67f7438f2a5f1b6e03974076..a22913868525a9d0d6fd5a2eff0269a895e3b3b3 100644 (file)
--- a/test/sql/test_from_linter.py
+++ b/test/sql/test_from_linter.py
@@ -281,6 +281,18 @@ class TestFindUnmatchingFroms(fixtures.TablesTest):
         froms, start = find_unmatching_froms(query)
         assert not froms
 
+    def test_join_on_true_muti_levels(self):
+        """test #6886"""
+        # test that a join(a, b).join(c) counts b->c as an edge even if there
+        # isn't actually a join condition.  this essentially allows a cartesian
+        # product to be added explicitly.
+
+        query = select(self.a, self.b, self.c).select_from(
+            self.a.join(self.b, true()).join(self.c, true())
+        )
+        froms, start = find_unmatching_froms(query)
+        assert not froms
+
     def test_right_nested_join_with_an_issue(self):
         query = (
             select(self.a)

diff --git a/test/sql/test_types.py b/test/sql/test_types.py
index 228d9fe3bd40e1f74cb9fe3989e1a72cbd6a02b4..261c3ed10d28f99af693b690a86304e0ce2cf586 100644 (file)
--- a/test/sql/test_types.py
+++ b/test/sql/test_types.py
@@ -2560,7 +2560,7 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest):
         (True, "omit_alias"), (False, "with_alias"), id_="ai", argnames="omit"
     )
     @testing.provide_metadata
-    @testing.skip_if('mysql < 8')
+    @testing.skip_if("mysql < 8")
     def test_duplicate_values_accepted(self, native, omit):
         foo_enum = pep435_enum("foo_enum")
         foo_enum("one", 1, "two")