From: Mike Bayer Date: Sun, 15 Aug 2021 16:21:13 +0000 (-0400) Subject: fix linter JOIN logic; fix PostgreSQL ARRAY op comparison X-Git-Tag: rel_1_4_23~7^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b987465332ab9fdbab9ad85435919a2967004b12;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..b4899912b8 --- /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 d8e54ae0c4..1b036ac323 100644 --- 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 861b0be164..94a0b42017 100644 --- 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 d2c81e63f8..0b56d89876 100644 --- 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 be049f5eb1..d3b2c20954 100644 --- 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 9e0ededecc..a229138685 100644 --- 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 228d9fe3bd..261c3ed10d 100644 --- 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")