From 74f9d5163f4857475236bebec9ef0d65ac224886 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 28 Jan 2021 14:53:02 -0500 Subject: [PATCH] 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 --- doc/build/changelog/unreleased_14/5888.rst | 16 +++++ lib/sqlalchemy/sql/compiler.py | 14 +++-- lib/sqlalchemy/testing/assertions.py | 3 + test/sql/test_compiler.py | 68 ++++++++++++++++++++++ test/sql/test_insert.py | 23 +++++--- 5 files changed, 113 insertions(+), 11 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/5888.rst diff --git a/doc/build/changelog/unreleased_14/5888.rst b/doc/build/changelog/unreleased_14/5888.rst new file mode 100644 index 0000000000..53e8a5dcb5 --- /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 aabc257eb5..353de2c480 100644 --- 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 f2ed91b793..79125e1f16 100644 --- 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 fea75d6791..2a543aa613 100644 --- 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 541ffc4e96..a128db8a9d 100644 --- 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 -- 2.47.2