From 7b53d9962f438cebaf82e95b3c8304578545c132 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Wed, 15 Aug 2018 17:11:14 -0400 Subject: [PATCH] Add concept of "implicit boolean", treat as native Fixed issue that is closely related to :ticket:`3639` where an expression rendered in a boolean context on a non-native boolean backend would be compared to 1/0 even though it is already an implcitly boolean expression, when :meth:`.ColumnElement.self_group` were used. While this does not affect the user-friendly backends (MySQL, SQLite) it was not handled by Oracle (and possibly SQL Server). Whether or not the expression is implicitly boolean on any database is now determined up front as an additional check to not generate the integer comparison within the compliation of the statement. Fixes: #4320 Change-Id: Iae0a65e5c01bd576e64733c3651e1e1a1a1b240c (cherry picked from commit 462ccd9ff18d2e428b20ec3f596391a275472140) --- doc/build/changelog/unreleased_12/4320.rst | 13 +++++++++ lib/sqlalchemy/sql/compiler.py | 6 ++-- lib/sqlalchemy/sql/elements.py | 21 +++++++++++++ lib/sqlalchemy/sql/operators.py | 6 ++++ lib/sqlalchemy/sql/selectable.py | 1 + test/sql/test_operators.py | 34 +++++++++++++++++++++- 6 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 doc/build/changelog/unreleased_12/4320.rst diff --git a/doc/build/changelog/unreleased_12/4320.rst b/doc/build/changelog/unreleased_12/4320.rst new file mode 100644 index 0000000000..b00a914dd2 --- /dev/null +++ b/doc/build/changelog/unreleased_12/4320.rst @@ -0,0 +1,13 @@ +.. change:: + :tags: bug, sql + :tickets: 4320 + + Fixed issue that is closely related to :ticket:`3639` where an expression + rendered in a boolean context on a non-native boolean backend would + be compared to 1/0 even though it is already an implcitly boolean + expression, when :meth:`.ColumnElement.self_group` were used. While this + does not affect the user-friendly backends (MySQL, SQLite) it was not + handled by Oracle (and possibly SQL Server). Whether or not the + expression is implicitly boolean on any database is now determined + up front as an additional check to not generate the integer comparison + within the compliation of the statement. diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 80df2fcb59..d93e08ece4 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -1019,13 +1019,15 @@ class SQLCompiler(Compiled): "Unary expression has no operator or modifier") def visit_istrue_unary_operator(self, element, operator, **kw): - if self.dialect.supports_native_boolean: + if element._is_implicitly_boolean or \ + self.dialect.supports_native_boolean: return self.process(element.element, **kw) else: return "%s = 1" % self.process(element.element, **kw) def visit_isfalse_unary_operator(self, element, operator, **kw): - if self.dialect.supports_native_boolean: + if element._is_implicitly_boolean or \ + self.dialect.supports_native_boolean: return "NOT %s" % self.process(element.element, **kw) else: return "%s = 0" % self.process(element.element, **kw) diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index 2e8c39f3b2..2a6fa323cf 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -641,6 +641,8 @@ class ColumnElement(operators.ColumnOperators, ClauseElement): """A flag that can be flipped to prevent a column from being resolvable by string label name.""" + _is_implicitly_boolean = False + _alt_names = () def self_group(self, against=None): @@ -1229,6 +1231,7 @@ class TextClause(Executable, ClauseElement): _execution_options = \ Executable._execution_options.union( {'autocommit': PARSE_AUTOCOMMIT}) + _is_implicitly_boolean = False @property def _select_iterable(self): @@ -1813,6 +1816,7 @@ class ClauseList(ClauseElement): self.clauses = [ text_converter(clause) for clause in clauses] + self._is_implicitly_boolean = operators.is_boolean(self.operator) def __iter__(self): return iter(self.clauses) @@ -1915,6 +1919,7 @@ class BooleanClauseList(ClauseList, ColumnElement): self.operator = operator self.group_contents = True self.type = type_api.BOOLEANTYPE + self._is_implicitly_boolean = True return self @classmethod @@ -2923,6 +2928,7 @@ class AsBoolean(UnaryExpression): self.negate = negate self.modifier = None self.wraps_column_expression = True + self._is_implicitly_boolean = element._is_implicitly_boolean def self_group(self, against=None): return self @@ -2950,6 +2956,12 @@ class BinaryExpression(ColumnElement): __visit_name__ = 'binary' + _is_implicitly_boolean = True + """Indicates that any database will know this is a boolean expression + even if the database does not have an explicit boolean datatype. + + """ + def __init__(self, left, right, operator, type_=None, negate=None, modifiers=None): # allow compatibility with libraries that @@ -2962,6 +2974,7 @@ class BinaryExpression(ColumnElement): self.operator = operator self.type = type_api.to_instance(type_) self.negate = negate + self._is_implicitly_boolean = operators.is_boolean(operator) if modifiers is None: self.modifiers = {} @@ -3066,6 +3079,10 @@ class Grouping(ColumnElement): def self_group(self, against=None): return self + @util.memoized_property + def _is_implicitly_boolean(self): + return self.element._is_implicitly_boolean + @property def _key_label(self): return self._label @@ -3550,6 +3567,10 @@ class Label(ColumnElement): def __reduce__(self): return self.__class__, (self.name, self._element, self._type) + @util.memoized_property + def _is_implicitly_boolean(self): + return self.element._is_implicitly_boolean + @util.memoized_property def _allow_label_resolve(self): return self.element._allow_label_resolve diff --git a/lib/sqlalchemy/sql/operators.py b/lib/sqlalchemy/sql/operators.py index 11d1954550..02a7a58eff 100644 --- a/lib/sqlalchemy/sql/operators.py +++ b/lib/sqlalchemy/sql/operators.py @@ -1281,6 +1281,12 @@ def is_natural_self_precedent(op): return op in _natural_self_precedent or \ isinstance(op, custom_op) and op.natural_self_precedent +_booleans = (inv, istrue, isfalse, and_, or_) + + +def is_boolean(op): + return is_comparison(op) or op in _booleans + _mirror = { gt: lt, ge: le, diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py index 04f6c086d1..5b665829b6 100644 --- a/lib/sqlalchemy/sql/selectable.py +++ b/lib/sqlalchemy/sql/selectable.py @@ -3576,6 +3576,7 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect): class ScalarSelect(Generative, Grouping): _from_objects = [] _is_from_container = True + _is_implicitly_boolean = False def __init__(self, element): self.element = element diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py index f4b3cf5840..00961a2e84 100644 --- a/test/sql/test_operators.py +++ b/test/sql/test_operators.py @@ -2105,9 +2105,41 @@ class NegationTest(fixtures.TestBase, testing.AssertsCompiledSQL): self.assert_compile( expr, - "NOT (mytable.myid = :myid_1 OR mytable.myid = :myid_2)" + "NOT (mytable.myid = :myid_1 OR mytable.myid = :myid_2)", + dialect=default.DefaultDialect(supports_native_boolean=False) ) + def test_negate_operator_self_group(self): + orig_expr = or_( + self.table1.c.myid == 1, self.table1.c.myid == 2).self_group() + expr = not_(orig_expr) + is_not_(expr, orig_expr) + + self.assert_compile( + expr, + "NOT (mytable.myid = :myid_1 OR mytable.myid = :myid_2)", + dialect=default.DefaultDialect(supports_native_boolean=False) + ) + + def test_implicitly_boolean(self): + # test for expressions that the database always considers as boolean + # even if there is no boolean datatype. + assert not self.table1.c.myid._is_implicitly_boolean + assert (self.table1.c.myid == 5)._is_implicitly_boolean + assert (self.table1.c.myid == 5).self_group()._is_implicitly_boolean + assert (self.table1.c.myid == 5).label('x')._is_implicitly_boolean + assert not_(self.table1.c.myid == 5)._is_implicitly_boolean + assert or_( + self.table1.c.myid == 5, self.table1.c.myid == 7 + )._is_implicitly_boolean + assert not column('x', Boolean)._is_implicitly_boolean + assert not (self.table1.c.myid + 5)._is_implicitly_boolean + assert not not_(column('x', Boolean))._is_implicitly_boolean + assert not select([self.table1.c.myid]).\ + as_scalar()._is_implicitly_boolean + assert not text("x = y")._is_implicitly_boolean + assert not literal_column("x = y")._is_implicitly_boolean + class LikeTest(fixtures.TestBase, testing.AssertsCompiledSQL): __dialect__ = 'default' -- 2.47.2