fix(render): keep un-aliased NULL unwrapped so NOT survives negation

mindsdb/utilities/render/sqlalchemy_render.py

@@ -9,7 +9,7 @@
 from sqlalchemy.dialects import mysql, postgresql, sqlite, mssql, oracle
 from sqlalchemy.schema import CreateTable, DropTable
 from sqlalchemy.sql import operators, ColumnElement, functions as sa_fnc
-from sqlalchemy.sql.expression import ClauseElement
+from sqlalchemy.sql.expression import ClauseElement, UnaryExpression
 
 from mindsdb_sql_parser import ast
 
@@ -284,6 +284,22 @@ def to_expression(self, t):
             arg1 = self.to_expression(t.args[1])
 
             op = t.op.lower()
+            # Keep bare NULL confined to IS predicates. Other expressions need
+            # the labeled literal: SQLAlchemy rewrites `= sa.null()` to IS NULL
+            # and rejects ordinary comparisons such as `> sa.null()`.
+            if op in ("is", "is not"):
+                if isinstance(t.args[0], ast.Constant) and t.args[0].value is None and not t.args[0].alias:
+                    arg0 = sa.null()
+                if isinstance(t.args[1], ast.Constant) and t.args[1].value is None and not t.args[1].alias:
+                    arg1 = sa.null()
+                elif (
+                    self.dialect.name == "sqlite"
+                    and isinstance(t.args[1], ast.Constant)
+                    and isinstance(t.args[1].value, bool)
+                ):
+                    # SQLite's IS TRUE/FALSE tests truthiness, unlike IS 1/0.
+                    # Preserve the keyword only in this right-hand position.
+                    arg1 = sa.literal_column("TRUE" if t.args[1].value else "FALSE", type_=sa.Boolean())
             if op in ("in", "not in"):
                 if t.args[1].parentheses:
                     arg1 = [arg1]
@@ -324,10 +340,21 @@ def to_expression(self, t):
                 "NOT": "__invert__",
                 "-": "__neg__",
             }
-            arg = self.to_expression(t.args[0])
-
-            method = opmap[t.op.upper()]
-            col = getattr(arg, method)()
+            operand = t.args[0]
+            arg = self.to_expression(operand)
+
+            if (
+                t.op.upper() == "NOT"
+                and isinstance(operand, ast.BinaryOperation)
+                and operand.op.lower() in ("is", "is not")
+            ):
+                # SQLAlchemy's inversion can drop NOT for expression operands.
+                # Preserve it explicitly: even swapping IS / IS NOT is not
+                # equivalent for PostgreSQL row-valued NULL predicates.
+                col = UnaryExpression(arg.self_group(), operator=operators.inv, type_=sa.Boolean())
+            else:
+                method = opmap[t.op.upper()]
+                col = getattr(arg, method)()
             if t.alias:
                 alias = self.get_alias(t.alias)
                 col = col.label(alias)

tests/unit/render/test_sqlalchemyrender.py

@@ -1,6 +1,11 @@
 import datetime as dt
+import sqlite3
+from contextlib import closing
 from textwrap import dedent
 
+import duckdb
+import pytest
+
 from mindsdb_sql_parser.ast import (
     Identifier,
     Select,
@@ -13,6 +18,7 @@
     Insert,
 )
 from mindsdb_sql_parser import parse_sql
+from mindsdb.interfaces.query_context.last_query import LastQuery
 from mindsdb.utilities.render.sqlalchemy_render import SqlalchemyRender
 
 
@@ -235,3 +241,229 @@ def test_mixed_join(self):
         """).strip()
 
         assert rendered.replace("\n", "") == expected.replace("\n", " ")
+
+
+class TestNullPredicateRendering:
+    """Regression tests: negated null predicates must not lose their NOT.
+
+    Wrapping an un-aliased NULL constant in a Label turns `x IS NULL` into a
+    bind-param comparison, which defeats SQLAlchemy's negate optimization, so
+    `NOT (x IS NULL)` compiled identical to `x IS NULL` (issue #12491).
+    """
+
+    def test_not_is_null_is_preserved(self):
+        rendered = SqlalchemyRender("mysql").get_string(
+            parse_sql("SELECT * FROM t WHERE NOT (x IS NULL)"), with_failback=False
+        )
+        assert str(parse_sql(rendered)) == str(parse_sql("SELECT * FROM t WHERE NOT (x IS NULL)"))
+
+    def test_not_is_not_null_is_preserved(self):
+        rendered = SqlalchemyRender("mysql").get_string(
+            parse_sql("SELECT * FROM t WHERE NOT (x IS NOT NULL)"), with_failback=False
+        )
+        assert str(parse_sql(rendered)) == str(parse_sql("SELECT * FROM t WHERE NOT (x IS NOT NULL)"))
+
+    def test_not_is_null_without_parens_is_preserved(self):
+        rendered = SqlalchemyRender("mysql").get_string(
+            parse_sql("SELECT * FROM t WHERE NOT x IS NULL"), with_failback=False
+        )
+        assert str(parse_sql(rendered)) == str(parse_sql("SELECT * FROM t WHERE NOT (x IS NULL)"))
+
+    def test_positive_null_predicates_unchanged(self):
+        for sql in (
+            "SELECT * FROM t WHERE x IS NULL",
+            "SELECT * FROM t WHERE x IS NOT NULL",
+        ):
+            rendered = SqlalchemyRender("mysql").get_string(parse_sql(sql), with_failback=False)
+            assert str(parse_sql(rendered)) == str(parse_sql(sql))
+
+    def test_aliased_null_in_select_keeps_label(self):
+        rendered = SqlalchemyRender("mysql").get_string(parse_sql("SELECT NULL AS nothing FROM t"), with_failback=False)
+        assert "AS nothing" in rendered
+
+    def test_injected_null_constant_in_comparison(self):
+        # LastQuery/update_step replace `last` and subselect params with
+        # Constant(None) placeholders before injecting values; under comparison
+        # operators those must render as plain NULL, not raise ArgumentError
+        # (which would silently failback to the invalid `a > None`).
+        query = parse_sql("SELECT * FROM t WHERE x > 0")
+        query.where.args[1] = Constant(value=None)
+        rendered = SqlalchemyRender("mysql").get_string(query, with_failback=False)
+        assert str(parse_sql(rendered)) == str(parse_sql("SELECT * FROM t WHERE x > NULL"))
+
+    def test_not_comparison_with_null_is_preserved(self):
+        rendered = SqlalchemyRender("mysql").get_string(
+            parse_sql("SELECT * FROM t WHERE NOT (x > NULL)"), with_failback=False
+        )
+        assert str(parse_sql(rendered)) == str(parse_sql("SELECT * FROM t WHERE x <= NULL"))
+
+    def test_eq_null_is_not_rewritten_to_is_null(self):
+        rendered = SqlalchemyRender("mysql").get_string(
+            parse_sql("SELECT * FROM t WHERE x = NULL"), with_failback=False
+        )
+        assert str(parse_sql(rendered)) == str(parse_sql("SELECT * FROM t WHERE x = NULL"))
+
+    def test_unaliased_null_in_select_keeps_null_label(self):
+        rendered = SqlalchemyRender("mysql").get_string(parse_sql("SELECT NULL FROM t"), with_failback=False)
+        assert "AS `NULL`" in rendered
+
+    @pytest.mark.parametrize("with_failback", [False, True])
+    def test_last_query_null_placeholder_and_value_injection(self, with_failback):
+        query = parse_sql("SELECT * FROM tasks WHERE a > last LIMIT 1")
+        last_query = LastQuery(query)
+        assert isinstance(query.where.args[1], Constant)
+        assert query.where.args[1].value is None
+        renderer = SqlalchemyRender("postgres")
+        rendered, params = renderer.get_exec_params(query, with_failback=with_failback)
+
+        assert params is None
+        assert "a > NULL" in rendered
+        with duckdb.connect(":memory:") as connection:
+            connection.execute("CREATE TABLE tasks(a INTEGER)")
+            connection.execute("INSERT INTO tasks VALUES (1), (2), (NULL)")
+            assert connection.execute(rendered).fetchall() == []
+
+            init_query, info = next(last_query.get_init_queries())
+            init_sql = renderer.get_string(init_query, with_failback=with_failback)
+            assert connection.execute(init_sql).fetchall() == [(2,)]
+
+            query = last_query.apply_values({info["table_name"]: {info["column_name"]: 2}})
+            rendered = renderer.get_string(query, with_failback=with_failback)
+            assert connection.execute(rendered).fetchall() == []
+            connection.execute("INSERT INTO tasks VALUES (3)")
+            assert connection.execute(rendered).fetchall() == [(3,)]
+
+
+class TestBooleanPredicateRendering:
+    @pytest.mark.parametrize("dialect", ["mysql", "postgres", "sqlite"])
+    @pytest.mark.parametrize("value", ["TRUE", "FALSE"])
+    @pytest.mark.parametrize(
+        "predicate, expected",
+        [
+            ("NOT (x IS {value})", "NOT (x IS {value})"),
+            ("NOT x IS {value}", "NOT (x IS {value})"),
+            ("NOT (x IS NOT {value})", "NOT (x IS NOT {value})"),
+            ("NOT (NOT (x IS {value}))", "NOT (NOT (x IS {value}))"),
+        ],
+    )
+    def test_negation_is_preserved(self, dialect, value, predicate, expected):
+        renderer = SqlalchemyRender(dialect)
+        rendered = renderer.get_string(
+            parse_sql(f"SELECT * FROM t WHERE {predicate.format(value=value)}"), with_failback=False
+        )
+        expected_sql = f"SELECT * FROM t WHERE {expected.format(value=value)}"
+        assert " ".join(rendered.split()).upper() == expected_sql.upper()
+
+    @pytest.mark.parametrize(
+        "predicate, expected",
+        [
+            ("NOT (x IS TRUE)", [(2,), (3,)]),
+            ("NOT (x IS FALSE)", [(1,), (3,)]),
+            ("NOT (x IS NOT TRUE)", [(1,)]),
+            ("NOT (x IS NOT FALSE)", [(2,)]),
+            ("NOT ((x IS TRUE) OR (x IS FALSE))", [(3,)]),
+            ("NOT ((id > 1) IS TRUE)", [(1,)]),
+            ("NOT (x IS UNKNOWN)", [(1,), (2,)]),
+            ("NOT (x IS NOT UNKNOWN)", [(3,)]),
+            ("NOT (x IS NULL)", [(1,), (2,)]),
+            ("NOT (x IS NOT NULL)", [(3,)]),
+            ("NOT (NOT (x IS TRUE))", [(1,)]),
+            ("NOT (NOT (NOT (x IS TRUE)))", [(2,), (3,)]),
+            ("NOT (x IS TRUE) AND id > 1", [(2,), (3,)]),
+            ("NOT (x IS TRUE) OR NOT (x IS FALSE)", [(1,), (2,), (3,)]),
+        ],
+    )
+    def test_negation_returns_correct_rows(self, predicate, expected):
+        rendered = SqlalchemyRender("postgres").get_string(
+            parse_sql(f"SELECT id FROM t WHERE {predicate} ORDER BY id"), with_failback=False
+        )
+        with duckdb.connect(":memory:") as connection:
+            connection.execute("CREATE TABLE t(id INTEGER, x BOOLEAN)")
+            connection.execute("INSERT INTO t VALUES (1, TRUE), (2, FALSE), (3, NULL)")
+            assert connection.execute(rendered).fetchall() == expected
+
+    def test_boolean_select_labels_are_preserved(self):
+        rendered = SqlalchemyRender("postgres").get_string(
+            parse_sql("SELECT TRUE, FALSE, TRUE AS yes, FALSE AS no"), with_failback=False
+        )
+        with duckdb.connect(":memory:") as connection:
+            result = connection.execute(rendered)
+            assert [column[0] for column in result.description] == ["True", "False", "yes", "no"]
+            assert result.fetchall() == [(True, False, True, False)]
+
+    @pytest.mark.parametrize("op", ["=", "!=", ">", "<"])
+    def test_boolean_comparisons_are_unchanged(self, op):
+        sql = f"SELECT * FROM t WHERE x {op} TRUE"
+        rendered = SqlalchemyRender("postgres").get_string(parse_sql(sql), with_failback=False)
+        assert str(parse_sql(rendered)) == str(parse_sql(sql))
+
+
+class TestIsPredicateNegation:
+    @pytest.mark.parametrize("dialect", ["mysql", "postgres", "sqlite", "mssql", "oracle"])
+    @pytest.mark.parametrize("op", ["IS", "IS NOT"])
+    def test_null_negation_is_explicit_across_dialects(self, dialect, op):
+        sql = f"SELECT * FROM t WHERE NOT (x {op} NULL)"
+        rendered = SqlalchemyRender(dialect).get_string(parse_sql(sql), with_failback=False)
+        assert " ".join(rendered.split()) == sql
+
+    @pytest.mark.parametrize("op", ["IS", "IS NOT"])
+    @pytest.mark.parametrize("negations", [0, 1, 2, 3])
+    @pytest.mark.parametrize(
+        "left, right",
+        [
+            ("x", "0"),
+            ("x", "1"),
+            ("x", "TRUE"),
+            ("x", "FALSE"),
+            ("x", "'TRUE'"),
+            ("x", "'abc'"),
+            ("x", "y"),
+            ("x", "(1 + 1)"),
+            ("x", "COALESCE(y, 0)"),
+            ("x", "(SELECT 1)"),
+            ("NULL", "x"),
+        ],
+    )
+    def test_sqlite_expression_operands_return_correct_rows(self, left, right, op, negations):
+        # SQLite permits arbitrary expressions on either side of IS / IS NOT.
+        predicate = "NOT (" * negations + f"{left} {op} {right}" + ")" * negations
+        sql = f"SELECT id FROM t WHERE {predicate} ORDER BY id"
+        rendered = SqlalchemyRender("sqlite").get_string(parse_sql(sql), with_failback=False)
+
+        with closing(sqlite3.connect(":memory:")) as connection:
+            connection.execute("CREATE TABLE t(id INTEGER, x, y)")
+            connection.executemany(
+                "INSERT INTO t VALUES (?, ?, ?)",
+                [
+                    (1, None, None),
+                    (2, 1, 1),
+                    (3, 2, 1),
+                    (4, "abc", "abc"),
+                    (5, "xyz", "abc"),
+                    (6, 0, 0),
+                    (7, -1, 0),
+                    (8, 0.5, 1),
+                    (9, "2", 2),
+                    (10, "", 0),
+                ],
+            )
+            expected = connection.execute(sql).fetchall()
+            assert connection.execute(rendered).fetchall() == expected
+
+    @pytest.mark.parametrize("op", ["IS", "IS NOT"])
+    def test_postgres_row_null_test_is_not_inverted(self, op):
+        # For a mixed-null PostgreSQL row, IS NULL and IS NOT NULL are both
+        # false. Swapping the operators is not equivalent to applying NOT.
+        sql = f"SELECT NOT (ROW(1, NULL) {op} NULL) AS result"
+        rendered = SqlalchemyRender("postgres").get_string(parse_sql(sql), with_failback=False)
+        assert " ".join(rendered.split()).upper() == sql.upper()
+
+    def test_negated_predicate_keeps_select_alias(self):
+        sql = "SELECT id, NOT (x IS NULL) AS is_present FROM t ORDER BY id"
+        rendered = SqlalchemyRender("postgres").get_string(parse_sql(sql), with_failback=False)
+        with duckdb.connect(":memory:") as connection:
+            connection.execute("CREATE TABLE t(id INTEGER, x BOOLEAN)")
+            connection.execute("INSERT INTO t VALUES (1, TRUE), (2, FALSE), (3, NULL)")
+            result = connection.execute(rendered)
+            assert [column[0] for column in result.description] == ["id", "is_present"]
+            assert result.fetchall() == [(1, True), (2, True), (3, False)]