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

diff --git a/doc/build/changelog/unreleased_12/4320.rst b/doc/build/changelog/unreleased_12/4320.rst
new file mode 100644 (file)
index 0000000..b00a914
--- /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 5fbc7d87f8c1f5c7f5a9a1a28abbf350a5109f3d..e45db428a507417accd953d5700cb30675b92f23 100644 (file)
--- 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 2e8c39f3b2c936c3eddd4ef30d09b635ffca10c1..2a6fa323cf754d93c2256d29f5dcecbc4a2b47e2 100644 (file)
--- 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 fd65979c52c461df4e3a7c3268154b1b71d3a5cd..a9f4e3e3ed2fa9c48dd1a96d046ae7451f498744 100644 (file)
--- a/lib/sqlalchemy/sql/operators.py
+++ b/lib/sqlalchemy/sql/operators.py
@@ -1315,6 +1315,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 04f6c086d17c907914ac12cd59e00482a77a7f02..5b665829b611ffbd2cbd35f481a2dd0244636689 100644 (file)
--- 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 f4b3cf58409f1b1f4f558943bf4c8d4b6b631cf7..00961a2e84aff92e080041e1d3d250d7b0c5ea43 100644 (file)
--- 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'