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 (file)
index 0000000..6071ce3
--- /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 36ed5ecf6fdaf7d99738e6472887b93454d00ced..1b8e26b8d45c76078035f50a697ecbb4203bf3c5 100644 (file)
--- 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 57ffdf86b531e46c925447a816c4afa1cb08c9dc..437a11a6077b87f0bf23b369a5c6d4bc6bfc0cc4 100644 (file)
--- 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 8133c21055e904f19e7081ad23c84bdb4628761b..1605033a023e14f3b3a1bcacbe5e1e073c2b63fa 100644 (file)
--- 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 4af1f65e3996db272ebe2a7fd36ce14ee302c015..eb8428569569fdae422e3de2e4331a319f38f4ad 100644 (file)
--- 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 8fe802bf3db54de853127c6db49ff212f1e2eb79..26714f6faefec16d9571658df4b034f3be5834f0 100644 (file)
--- 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",
         )