Render NULL for bindparam w/ None value/literal_binds, warn

Adjusted the "literal_binds" feature of :class:`_sql.Compiler` to render
NULL for a bound parameter that has ``None`` as the value, either
explicitly passed or omitted. The previous error message "bind parameter
without a renderable value" is removed, and a missing or ``None`` value
will now render NULL in all cases. Previously, rendering of NULL was
starting to happen for DML statements due to internal refactorings, but was
not explicitly part of test coverage, which it now is.

While no error is raised, when the context is within that of a column
comparison, and the operator is not "IS"/"IS NOT", a warning is emitted
that this is not generally useful from a SQL perspective.

Fixes: #5888
Change-Id: Id5939d8dbfb1156a9f8a7f7e76cf18327155331a

diff --git a/doc/build/changelog/unreleased_14/5888.rst b/doc/build/changelog/unreleased_14/5888.rst
new file mode 100644 (file)
index 0000000..53e8a5d
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/5888.rst
@@ -0,0 +1,16 @@
+.. change::
+    :tags: usecase, sql
+    :tickets: 5888
+
+    Adjusted the "literal_binds" feature of :class:`_sql.Compiler` to render
+    NULL for a bound parameter that has ``None`` as the value, either
+    explicitly passed or omitted. The previous error message "bind parameter
+    without a renderable value" is removed, and a missing or ``None`` value
+    will now render NULL in all cases. Previously, rendering of NULL was
+    starting to happen for DML statements due to internal refactorings, but was
+    not explicitly part of test coverage, which it now is.
+
+    While no error is raised, when the context is within that of a column
+    comparison, and the operator is not "IS"/"IS NOT", a warning is emitted
+    that this is not generally useful from a SQL perspective.
+

diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index aabc257eb5ebd74896557c227473149c4d2932ba..353de2c480a4bfca99c5b68b37763662e57b01fa 100644 (file)
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -2055,6 +2055,7 @@ class SQLCompiler(Compiled):
         _in_binary = kw.get("_in_binary", False)
 
         kw["_in_binary"] = True
+        kw["_binary_op"] = binary.operator
         text = (
             binary.left._compiler_dispatch(
                 self, eager_grouping=eager_grouping, **kw
@@ -2306,10 +2307,15 @@ class SQLCompiler(Compiled):
             value = render_literal_value
         else:
             if bindparam.value is None and bindparam.callable is None:
-                raise exc.CompileError(
-                    "Bind parameter '%s' without a "
-                    "renderable value not allowed here." % bindparam.key
-                )
+                op = kw.get("_binary_op", None)
+                if op and op not in (operators.is_, operators.is_not):
+                    util.warn_limited(
+                        "Bound parameter '%s' rendering literal NULL in a SQL "
+                        "expression; comparisons to NULL should not use "
+                        "operators outside of 'is' or 'is not'",
+                        (bindparam.key,),
+                    )
+                return self.process(sqltypes.NULLTYPE, **kw)
             value = bindparam.effective_value
 
         if bindparam.expanding:

diff --git a/lib/sqlalchemy/testing/assertions.py b/lib/sqlalchemy/testing/assertions.py
index f2ed91b79376daf8d8bc78acbf90b03aa306b79f..79125e1f16af904eaafc0bf36f279e56535aa885 100644 (file)
--- a/lib/sqlalchemy/testing/assertions.py
+++ b/lib/sqlalchemy/testing/assertions.py
@@ -388,6 +388,7 @@ class AssertsCompiledSQL(object):
         check_prefetch=None,
         use_default_dialect=False,
         allow_dialect_select=False,
+        supports_default_values=True,
         literal_binds=False,
         render_postcompile=False,
         schema_translate_map=None,
@@ -396,6 +397,7 @@ class AssertsCompiledSQL(object):
     ):
         if use_default_dialect:
             dialect = default.DefaultDialect()
+            dialect.supports_default_values = supports_default_values
         elif allow_dialect_select:
             dialect = None
         else:
@@ -406,6 +408,7 @@ class AssertsCompiledSQL(object):
                 dialect = config.db.dialect
             elif dialect == "default":
                 dialect = default.DefaultDialect()
+                dialect.supports_default_values = supports_default_values
             elif dialect == "default_enhanced":
                 dialect = default.StrCompileDialect()
             elif isinstance(dialect, util.string_types):

diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py
index fea75d6791cea42066711162753ed47d6f87dfcf..2a543aa613622399da537afd51820b4a0a77682e 100644 (file)
--- a/test/sql/test_compiler.py
+++ b/test/sql/test_compiler.py
@@ -4171,6 +4171,74 @@ class BindParameterTest(AssertsCompiledSQL, fixtures.TestBase):
             checkparams={"foo_1": 1, "foo_2": 2, "foo_3": 3},
         )
 
+    @testing.combinations(
+        (
+            select(table1.c.myid).where(
+                table1.c.myid == bindparam("x", value=None)
+            ),
+            "SELECT mytable.myid FROM mytable WHERE mytable.myid = NULL",
+            True,
+            None,
+        ),
+        (
+            select(table1.c.myid).where(table1.c.myid == None),
+            "SELECT mytable.myid FROM mytable WHERE mytable.myid IS NULL",
+            False,
+            None,
+        ),
+        (
+            select(table1.c.myid, None),
+            "SELECT mytable.myid, NULL AS anon_1 FROM mytable",
+            False,
+            None,
+        ),
+        (
+            select(table1.c.myid).where(
+                table1.c.myid.is_(bindparam("x", value=None))
+            ),
+            "SELECT mytable.myid FROM mytable WHERE mytable.myid IS NULL",
+            False,
+            None,
+        ),
+        (
+            # as of SQLAlchemy 1.4, values like these are considered to be
+            # SQL expressions up front, so it is coerced to null()
+            # immediately and no bindparam() is created
+            table1.insert().values({"myid": None}),
+            "INSERT INTO mytable (myid) VALUES (NULL)",
+            False,
+            None,
+        ),
+        (table1.insert(), "INSERT INTO mytable DEFAULT VALUES", False, {}),
+        (
+            table1.update().values({"myid": None}),
+            "UPDATE mytable SET myid=NULL",
+            False,
+            None,
+        ),
+        (
+            table1.update()
+            .where(table1.c.myid == bindparam("x"))
+            .values({"myid": None}),
+            "UPDATE mytable SET myid=NULL WHERE mytable.myid = NULL",
+            True,
+            None,
+        ),
+    )
+    def test_render_nulls_literal_binds(self, stmt, expected, warns, params):
+        if warns:
+            with testing.expect_warnings(
+                r"Bound parameter '.*?' rendering literal "
+                "NULL in a SQL expression"
+            ):
+                self.assert_compile(
+                    stmt, expected, literal_binds=True, params=params
+                )
+        else:
+            self.assert_compile(
+                stmt, expected, literal_binds=True, params=params
+            )
+
 
 class UnsupportedTest(fixtures.TestBase):
     def test_unsupported_element_str_visit_name(self):

diff --git a/test/sql/test_insert.py b/test/sql/test_insert.py
index 541ffc4e966780609102d6d557428804859e7308..a128db8a9d14dc0dcc5e99e05f9c6c3e0601b562 100644 (file)
--- a/test/sql/test_insert.py
+++ b/test/sql/test_insert.py
@@ -820,7 +820,10 @@ class InsertTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
             "Column 't.id' is marked as a member.*" "may not store NULL.$"
         ):
             self.assert_compile(
-                t.insert(), "INSERT INTO t () VALUES ()", params={}
+                t.insert(),
+                "INSERT INTO t () VALUES ()",
+                params={},
+                supports_default_values=False,
             )
 
 
@@ -928,17 +931,20 @@ class EmptyTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
         table1 = self.tables.mytable
 
         stmt = table1.insert().values({})  # hide from 2to3
-        self.assert_compile(stmt, "INSERT INTO mytable () VALUES ()")
+        self.assert_compile(
+            stmt,
+            "INSERT INTO mytable () VALUES ()",
+            supports_default_values=False,
+        )
 
     def test_supports_empty_insert_true(self):
         table1 = self.tables.mytable
 
-        dialect = default.DefaultDialect()
-        dialect.supports_empty_insert = dialect.supports_default_values = True
-
         stmt = table1.insert().values({})
         self.assert_compile(
-            stmt, "INSERT INTO mytable DEFAULT VALUES", dialect=dialect
+            stmt,
+            "INSERT INTO mytable DEFAULT VALUES",
+            supports_default_values=True,
         )
 
     def test_supports_empty_insert_true_executemany_mode(self):
@@ -977,7 +983,10 @@ class EmptyTest(_InsertTestBase, fixtures.TablesTest, AssertsCompiledSQL):
         ins = table1.insert().values(collection)
 
         self.assert_compile(
-            ins, "INSERT INTO mytable () VALUES ()", checkparams={}
+            ins,
+            "INSERT INTO mytable () VALUES ()",
+            checkparams={},
+            supports_default_values=False,
         )
 
         # empty dict populates on next values call