From 1de64504d8e68e2c0d14669c7638cf6f6d74973f Mon Sep 17 00:00:00 2001 From: Federico Caselli Date: Thu, 23 Jan 2020 17:51:38 -0500 Subject: Deprecate empty or_() and and_() Creating an :func:`.and_` or :func:`.or_` construct with no arguments or empty ``*args`` will now emit a deprecation warning, as the SQL produced is a no-op (i.e. it renders as a blank string). This behavior is considered to be non-intuitive, so for empty or possibly empty :func:`.and_` or :func:`.or_` constructs, an appropriate default boolean should be included, such as ``and_(True, *args)`` or ``or_(False, *args)``. As has been the case for many major versions of SQLAlchemy, these particular boolean values will not render if the ``*args`` portion is non-empty. As there are some internal cases where an empty and_() construct is used in order to build an optional WHERE expression, a private utility function is added to suit this use case. Co-authored-by: Mike Bayer Fixes: #5054 Closes: #5062 Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/5062 Pull-request-sha: 5ca2f27281977d74e390148c0fb8deaa0e0e4ad9 Change-Id: I599b9c8befa64d9a59a35ad7dd84ff400e3aa647 --- test/sql/test_compiler.py | 22 ++++++++++++--- test/sql/test_delete.py | 15 ++++++----- test/sql/test_deprecations.py | 12 ++++++++- test/sql/test_operators.py | 63 ++++++++++++++++++++++++++++++++++++++++--- test/sql/test_selectable.py | 4 ++- test/sql/test_update.py | 12 ++++++--- 6 files changed, 109 insertions(+), 19 deletions(-) (limited to 'test/sql') diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py index b49fb455c..b53acf61e 100644 --- a/test/sql/test_compiler.py +++ b/test/sql/test_compiler.py @@ -70,7 +70,9 @@ from sqlalchemy.ext.compiler import compiles from sqlalchemy.sql import column from sqlalchemy.sql import compiler from sqlalchemy.sql import label +from sqlalchemy.sql import operators from sqlalchemy.sql import table +from sqlalchemy.sql.elements import BooleanClauseList from sqlalchemy.sql.expression import ClauseList from sqlalchemy.sql.expression import HasPrefixes from sqlalchemy.testing import assert_raises @@ -1391,12 +1393,22 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): select([t]).where(and_(or_(t.c.x == 12, and_(or_(t.c.x == 8))))), "SELECT t.x FROM t WHERE t.x = :x_1 OR t.x = :x_2", ) + self.assert_compile( + select([t]).where( + and_(or_(or_(t.c.x == 12), and_(or_(and_(t.c.x == 8))))) + ), + "SELECT t.x FROM t WHERE t.x = :x_1 OR t.x = :x_2", + ) self.assert_compile( select([t]).where( and_( or_( or_(t.c.x == 12), - and_(or_(), or_(and_(t.c.x == 8)), and_()), + and_( + BooleanClauseList._construct_raw(operators.or_), + or_(and_(t.c.x == 8)), + BooleanClauseList._construct_raw(operators.and_), + ), ) ) ), @@ -1451,11 +1463,15 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): def test_where_empty(self): self.assert_compile( - select([table1.c.myid]).where(and_()), + select([table1.c.myid]).where( + BooleanClauseList._construct_raw(operators.and_) + ), "SELECT mytable.myid FROM mytable", ) self.assert_compile( - select([table1.c.myid]).where(or_()), + select([table1.c.myid]).where( + BooleanClauseList._construct_raw(operators.or_) + ), "SELECT mytable.myid FROM mytable", ) diff --git a/test/sql/test_delete.py b/test/sql/test_delete.py index 1f4c49c56..5dcc0d112 100644 --- a/test/sql/test_delete.py +++ b/test/sql/test_delete.py @@ -15,6 +15,7 @@ from sqlalchemy.engine import default from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import eq_ +from sqlalchemy.testing import expect_deprecated from sqlalchemy.testing import fixtures from sqlalchemy.testing.schema import Column from sqlalchemy.testing.schema import Table @@ -77,12 +78,14 @@ class DeleteTest(_DeleteTestBase, fixtures.TablesTest, AssertsCompiledSQL): def test_where_empty(self): table1 = self.tables.mytable - self.assert_compile( - table1.delete().where(and_()), "DELETE FROM mytable" - ) - self.assert_compile( - table1.delete().where(or_()), "DELETE FROM mytable" - ) + with expect_deprecated(): + self.assert_compile( + table1.delete().where(and_()), "DELETE FROM mytable" + ) + with expect_deprecated(): + self.assert_compile( + table1.delete().where(or_()), "DELETE FROM mytable" + ) def test_prefix_with(self): table1 = self.tables.mytable diff --git a/test/sql/test_deprecations.py b/test/sql/test_deprecations.py index ec7b2ac38..4e88dcdb8 100644 --- a/test/sql/test_deprecations.py +++ b/test/sql/test_deprecations.py @@ -1,6 +1,7 @@ #! coding: utf-8 from sqlalchemy import alias +from sqlalchemy import and_ from sqlalchemy import bindparam from sqlalchemy import CHAR from sqlalchemy import column @@ -14,6 +15,7 @@ from sqlalchemy import join from sqlalchemy import literal_column from sqlalchemy import MetaData from sqlalchemy import null +from sqlalchemy import or_ from sqlalchemy import select from sqlalchemy import sql from sqlalchemy import String @@ -42,7 +44,7 @@ from sqlalchemy.testing.schema import Column from sqlalchemy.testing.schema import Table -class DeprecationWarningsTest(fixtures.TestBase): +class DeprecationWarningsTest(fixtures.TestBase, AssertsCompiledSQL): __backend__ = True def test_ident_preparer_force(self): @@ -122,6 +124,14 @@ class DeprecationWarningsTest(fixtures.TestBase): ): select([column("x")], for_update=True) + def test_empty_and_or(self): + with testing.expect_deprecated( + r"Invoking and_\(\) without arguments is deprecated, and " + r"will be disallowed in a future release. For an empty " + r"and_\(\) construct, use and_\(True, \*args\)" + ): + self.assert_compile(or_(and_()), "") + class ConvertUnicodeDeprecationTest(fixtures.TestBase): diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py index a90b03b38..d3afc2dee 100644 --- a/test/sql/test_operators.py +++ b/test/sql/test_operators.py @@ -42,6 +42,7 @@ from sqlalchemy.sql import sqltypes from sqlalchemy.sql import table from sqlalchemy.sql import true from sqlalchemy.sql.elements import BindParameter +from sqlalchemy.sql.elements import BooleanClauseList from sqlalchemy.sql.elements import Label from sqlalchemy.sql.expression import BinaryExpression from sqlalchemy.sql.expression import ClauseList @@ -51,6 +52,7 @@ from sqlalchemy.sql.expression import tuple_ from sqlalchemy.sql.expression import UnaryExpression from sqlalchemy.sql.expression import union from sqlalchemy.testing import assert_raises_message +from sqlalchemy.testing import combinations from sqlalchemy.testing import eq_ from sqlalchemy.testing import expect_warnings from sqlalchemy.testing import fixtures @@ -1062,14 +1064,67 @@ class ConjunctionTest(fixtures.TestBase, testing.AssertsCompiledSQL): __dialect__ = default.DefaultDialect(supports_native_boolean=True) - def test_one(self): + def test_single_bool_one(self): self.assert_compile(~and_(true()), "false") - def test_two(self): + def test_single_bool_two(self): + self.assert_compile(~and_(True), "false") + + def test_single_bool_three(self): self.assert_compile(or_(~and_(true())), "false") - def test_three(self): - self.assert_compile(or_(and_()), "") + def test_single_bool_four(self): + self.assert_compile(~or_(false()), "true") + + def test_single_bool_five(self): + self.assert_compile(~or_(False), "true") + + def test_single_bool_six(self): + self.assert_compile(and_(~or_(false())), "true") + + def test_single_bool_seven(self): + self.assert_compile(and_(True), "true") + + def test_single_bool_eight(self): + self.assert_compile(or_(False), "false") + + def test_single_bool_nine(self): + self.assert_compile( + and_(True), + "1 = 1", + dialect=default.DefaultDialect(supports_native_boolean=False), + ) + + def test_single_bool_ten(self): + self.assert_compile( + or_(False), + "0 = 1", + dialect=default.DefaultDialect(supports_native_boolean=False), + ) + + @combinations((and_, "and_", "True"), (or_, "or_", "False")) + def test_empty_clauses(self, op, str_op, str_continue): + # these warning classes will change to ArgumentError when the + # deprecated behavior is disabled + assert_raises_message( + exc.SADeprecationWarning, + r"Invoking %(str_op)s\(\) without arguments is deprecated, and " + r"will be disallowed in a future release. For an empty " + r"%(str_op)s\(\) construct, use " + r"%(str_op)s\(%(str_continue)s, \*args\)\." + % {"str_op": str_op, "str_continue": str_continue}, + op, + ) + + def test_empty_and_raw(self): + self.assert_compile( + BooleanClauseList._construct_raw(operators.and_), "" + ) + + def test_empty_or_raw(self): + self.assert_compile( + BooleanClauseList._construct_raw(operators.and_), "" + ) def test_four(self): x = column("x") diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py index be7e28b5d..c3655efd2 100644 --- a/test/sql/test_selectable.py +++ b/test/sql/test_selectable.py @@ -32,6 +32,7 @@ from sqlalchemy.sql import Alias from sqlalchemy.sql import base from sqlalchemy.sql import column from sqlalchemy.sql import elements +from sqlalchemy.sql import operators from sqlalchemy.sql import table from sqlalchemy.sql import util as sql_util from sqlalchemy.sql import visitors @@ -2678,7 +2679,8 @@ class ReprTest(fixtures.TestBase): elements.True_(), elements.False_(), elements.ClauseList(), - elements.BooleanClauseList.and_(), + elements.BooleanClauseList._construct_raw(operators.and_), + elements.BooleanClauseList._construct_raw(operators.or_), elements.Tuple(), elements.Case([]), elements.Extract("foo", column("x")), diff --git a/test/sql/test_update.py b/test/sql/test_update.py index e625b7d9c..0313db832 100644 --- a/test/sql/test_update.py +++ b/test/sql/test_update.py @@ -1,4 +1,3 @@ -from sqlalchemy import and_ from sqlalchemy import bindparam from sqlalchemy import column from sqlalchemy import exc @@ -8,7 +7,6 @@ from sqlalchemy import func from sqlalchemy import Integer from sqlalchemy import literal from sqlalchemy import MetaData -from sqlalchemy import or_ from sqlalchemy import select from sqlalchemy import String from sqlalchemy import table @@ -18,6 +16,8 @@ from sqlalchemy import update from sqlalchemy import util from sqlalchemy.dialects import mysql from sqlalchemy.engine import default +from sqlalchemy.sql import operators +from sqlalchemy.sql.elements import BooleanClauseList from sqlalchemy.testing import assert_raises from sqlalchemy.testing import assert_raises_message from sqlalchemy.testing import AssertsCompiledSQL @@ -634,12 +634,16 @@ class UpdateTest(_UpdateFromTestBase, fixtures.TablesTest, AssertsCompiledSQL): def test_where_empty(self): table1 = self.tables.mytable self.assert_compile( - table1.update().where(and_()), + table1.update().where( + BooleanClauseList._construct_raw(operators.and_) + ), "UPDATE mytable SET myid=:myid, name=:name, " "description=:description", ) self.assert_compile( - table1.update().where(or_()), + table1.update().where( + BooleanClauseList._construct_raw(operators.or_) + ), "UPDATE mytable SET myid=:myid, name=:name, " "description=:description", ) -- cgit v1.2.1