From: Mike Bayer Date: Wed, 5 May 2021 12:38:54 +0000 (-0400) Subject: Parenthesize for empty not in X-Git-Tag: rel_1_4_14~6^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5af854606b6aabb7eb07311572acafe01cc73737;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Parenthesize for empty not in Fixed regression caused by the "empty in" change just made in :ticket:`6397` 1.4.12 where the expression needs to be parenthesized for the "not in" use case, otherwise the condition will interfere with the other filtering criteria. also amends StrSQLCompiler to use the newer "empty IN" style for its compilation process. Fixes: #6428 Change-Id: I182a552fc0d3065a9e38c0f4ece2deb143735c36 --- diff --git a/doc/build/changelog/unreleased_14/6428.rst b/doc/build/changelog/unreleased_14/6428.rst new file mode 100644 index 0000000000..6071ce33a1 --- /dev/null +++ b/doc/build/changelog/unreleased_14/6428.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, regression, sql + :tickets: 6428 + + Fixed regression caused by the "empty in" change just made in + :ticket:`6397` 1.4.12 where the expression needs to be parenthesized for + the "not in" use case, otherwise the condition will interfere with the + other filtering criteria. + diff --git a/doc/build/core/operators.rst b/doc/build/core/operators.rst index 36ed5ecf6f..1b8e26b8d4 100644 --- a/doc/build/core/operators.rst +++ b/doc/build/core/operators.rst @@ -212,12 +212,12 @@ NOT IN "NOT IN" is available via the :meth:`_sql.ColumnOperators.not_in` operator:: >>> print(column('x').not_in([1, 2, 3])) - x NOT IN ([POSTCOMPILE_x_1]) + (x NOT IN ([POSTCOMPILE_x_1])) This is typically more easily available by negating with the ``~`` operator:: >>> print(~column('x').in_([1, 2, 3])) - x NOT IN ([POSTCOMPILE_x_1]) + (x NOT IN ([POSTCOMPILE_x_1])) Tuple IN Expressions ~~~~~~~~~~~~~~~~~~~~ diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 57ffdf86b5..437a11a607 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -1908,7 +1908,9 @@ class SQLCompiler(Compiled): return self._render_in_expr_w_bindparam(binary, operator, **kw) def visit_not_in_op_binary(self, binary, operator, **kw): - return self._render_in_expr_w_bindparam(binary, operator, **kw) + return "(%s)" % self._render_in_expr_w_bindparam( + binary, operator, **kw + ) def _render_in_expr_w_bindparam(self, binary, operator, **kw): opstring = OPERATORS[operator] @@ -1957,10 +1959,13 @@ class SQLCompiler(Compiled): if parameter.type._is_tuple_type: replacement_expression = ( "VALUES " if self.dialect.tuple_in_values else "" - ) + self.visit_empty_set_expr(parameter.type.types) + ) + self.visit_empty_set_op_expr( + parameter.type.types, parameter.expand_op + ) + else: - replacement_expression = self.visit_empty_set_expr( - [parameter.type] + replacement_expression = self.visit_empty_set_op_expr( + [parameter.type], parameter.expand_op ) elif isinstance(values[0], (tuple, list)): @@ -3939,9 +3944,6 @@ class StrSQLCompiler(SQLCompiler): for t in extra_froms ) - def visit_empty_set_op_expr(self, type_, expand_op): - return self.visit_empty_set_expr(type_) - def visit_empty_set_expr(self, type_): return "SELECT 1 WHERE 1!=1" diff --git a/lib/sqlalchemy/testing/suite/test_select.py b/lib/sqlalchemy/testing/suite/test_select.py index 8133c21055..1605033a02 100644 --- a/lib/sqlalchemy/testing/suite/test_select.py +++ b/lib/sqlalchemy/testing/suite/test_select.py @@ -1110,6 +1110,26 @@ class ExpandingBoundInTest(fixtures.TablesTest): ) self._assert_result(stmt, [(2,), (3,), (4,)]) + def test_nonempty_in_plus_empty_notin(self): + table = self.tables.some_table + stmt = ( + select(table.c.id) + .where(table.c.x.in_([2, 3])) + .where(table.c.id.not_in([])) + .order_by(table.c.id) + ) + self._assert_result(stmt, [(2,), (3,)]) + + def test_empty_in_plus_notempty_notin(self): + table = self.tables.some_table + stmt = ( + select(table.c.id) + .where(table.c.x.in_([])) + .where(table.c.id.not_in([2, 3])) + .order_by(table.c.id) + ) + self._assert_result(stmt, []) + @testing.requires.tuple_in def test_bound_in_two_tuple_bindparam(self): table = self.tables.some_table diff --git a/test/sql/test_deprecations.py b/test/sql/test_deprecations.py index 4af1f65e39..eb84285695 100644 --- a/test/sql/test_deprecations.py +++ b/test/sql/test_deprecations.py @@ -2387,7 +2387,8 @@ class LegacyOperatorTest(AssertsCompiledSQL, fixtures.TestBase): self.assert_compile(column("x").isnot("foo"), "x IS NOT :x_1") self.assert_compile( - column("x").notin_(["foo", "bar"]), "x NOT IN ([POSTCOMPILE_x_1])" + column("x").notin_(["foo", "bar"]), + "(x NOT IN ([POSTCOMPILE_x_1]))", ) def test_issue_5429_operators(self): diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py index 8fe802bf3d..26714f6fae 100644 --- a/test/sql/test_operators.py +++ b/test/sql/test_operators.py @@ -1732,7 +1732,7 @@ class InTest(fixtures.TestBase, testing.AssertsCompiledSQL): def test_in_2(self): self.assert_compile( ~self.table1.c.myid.in_(["a"]), - "mytable.myid NOT IN ([POSTCOMPILE_myid_1])", + "(mytable.myid NOT IN ([POSTCOMPILE_myid_1]))", checkparams={"myid_1": ["a"]}, ) @@ -1854,8 +1854,8 @@ class InTest(fixtures.TestBase, testing.AssertsCompiledSQL): def test_in_21(self): self.assert_compile( ~self.table1.c.myid.in_(select(self.table2.c.otherid)), - "mytable.myid NOT IN " - "(SELECT myothertable.otherid FROM myothertable)", + "(mytable.myid NOT IN " + "(SELECT myothertable.otherid FROM myothertable))", ) def test_in_22(self): @@ -1944,18 +1944,31 @@ class InTest(fixtures.TestBase, testing.AssertsCompiledSQL): expr = t1.in_([(3, "hi", b"there"), (4, "Q", b"P")]) if not is_in: expr = ~expr - self.assert_compile( - expr, - "(a, b, c) %s ([POSTCOMPILE_param_1])" - % ("IN" if is_in else "NOT IN"), - checkparams={"param_1": [(3, "hi", b"there"), (4, "Q", b"P")]}, - ) - self.assert_compile( - expr, - "(a, b, c) %s ((3, 'hi', 'there'), (4, 'Q', 'P'))" - % ("IN" if is_in else "NOT IN"), - literal_binds=True, - ) + + if is_in: + self.assert_compile( + expr, + "(a, b, c) %s ([POSTCOMPILE_param_1])" + % ("IN" if is_in else "NOT IN"), + checkparams={"param_1": [(3, "hi", b"there"), (4, "Q", b"P")]}, + ) + self.assert_compile( + expr, + "(a, b, c) %s ((3, 'hi', 'there'), (4, 'Q', 'P'))" + % ("IN" if is_in else "NOT IN"), + literal_binds=True, + ) + else: + self.assert_compile( + expr, + "((a, b, c) NOT IN ([POSTCOMPILE_param_1]))", + checkparams={"param_1": [(3, "hi", b"there"), (4, "Q", b"P")]}, + ) + self.assert_compile( + expr, + "((a, b, c) NOT IN ((3, 'hi', 'there'), (4, 'Q', 'P')))", + literal_binds=True, + ) @testing.combinations(True, False) def test_in_empty_tuple(self, is_in): @@ -1966,19 +1979,61 @@ class InTest(fixtures.TestBase, testing.AssertsCompiledSQL): ) t1 = tuple_(a, b, c) expr = t1.in_([]) if is_in else t1.not_in([]) - self.assert_compile( - expr, - "(a, b, c) %s ([POSTCOMPILE_param_1])" - % ("IN" if is_in else "NOT IN"), - checkparams={"param_1": []}, - ) - self.assert_compile( - expr, - "(a, b, c) %s (SELECT 1 WHERE 1!=1)" - % ("IN" if is_in else "NOT IN"), - literal_binds=True, - dialect="default_enhanced", - ) + + if is_in: + self.assert_compile( + expr, + "(a, b, c) IN ([POSTCOMPILE_param_1])", + checkparams={"param_1": []}, + ) + self.assert_compile( + expr, + "(a, b, c) IN ((NULL, NULL, NULL)) AND (1 != 1)", + literal_binds=True, + dialect="default_enhanced", + ) + else: + self.assert_compile( + expr, + "((a, b, c) NOT IN ([POSTCOMPILE_param_1]))", + checkparams={"param_1": []}, + ) + self.assert_compile( + expr, + "((a, b, c) NOT IN ((NULL, NULL, NULL)) OR (1 = 1))", + literal_binds=True, + dialect="default_enhanced", + ) + + @testing.combinations(True, False) + def test_in_empty_single(self, is_in): + a = column("a", Integer) + expr = a.in_([]) if is_in else a.not_in([]) + + if is_in: + self.assert_compile( + expr, + "a IN ([POSTCOMPILE_a_1])", + checkparams={"a_1": []}, + ) + self.assert_compile( + expr, + "a IN (NULL) AND (1 != 1)", + literal_binds=True, + dialect="default_enhanced", + ) + else: + self.assert_compile( + expr, + "(a NOT IN ([POSTCOMPILE_a_1]))", + checkparams={"a_1": []}, + ) + self.assert_compile( + expr, + "(a NOT IN (NULL) OR (1 = 1))", + literal_binds=True, + dialect="default_enhanced", + ) def test_in_set(self): s = {1, 2, 3} @@ -2034,7 +2089,7 @@ class InTest(fixtures.TestBase, testing.AssertsCompiledSQL): self.assert_compile( expr, - "q IN (SELECT 1 WHERE 1!=1)", + "q IN (NULL) AND (1 != 1)", literal_binds=True, dialect="default_enhanced", )