- BinaryExpression now keeps track of "left" and "right" as passed in,

so that they can be compared in ``__nonzero__`` prior to their
self_group() step.  [ticket:2621]

4 files changed, 80 insertions, 4 deletions

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index 38aba26d8..a27e076f8 100644
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -7,6 +7,27 @@
     :version: 0.8.0b2
 
     .. change::
+        :tags: sql, bug
+        :tickets: 2621
+
+      Made an adjustment to the "boolean", (i.e. ``__nonzero__``)
+      evaluation of binary expressions, i.e. ``x1 == x2``, such
+      that the "auto-grouping" applied by :class:`.BinaryExpression`
+      in some cases won't get in the way of this comparison.
+      Previously, an expression like::
+
+        expr1 = mycolumn > 2
+        bool(expr1 == expr1)
+
+      Would evaulate as ``False``, even though this is an identity
+      comparison, because ``mycolumn > 2`` would be "grouped" before
+      being placed into the :class:`.BinaryExpression`, thus changing
+      its identity.   :class:`.BinaryExpression` now keeps track
+      of the "original" objects passed in.
+      Additionally the ``__nonzero__`` method now only returns if
+      the operator is ``==`` or ``!=`` - all others raise ``TypeError``.
+
+    .. change::
         :tags: firebird, bug
         :tickets: 2622
 
diff --git a/lib/sqlalchemy/sql/expression.py b/lib/sqlalchemy/sql/expression.py
index 3dc8dfea4..aa912a0f6 100644
--- a/lib/sqlalchemy/sql/expression.py
+++ b/lib/sqlalchemy/sql/expression.py
@@ -3723,6 +3723,7 @@ class BinaryExpression(ColumnElement):
         # refer to BinaryExpression directly and pass strings
         if isinstance(operator, basestring):
             operator = operators.custom_op(operator)
+        self._orig = (left, right)
         self.left = _literal_as_text(left).self_group(against=operator)
         self.right = _literal_as_text(right).self_group(against=operator)
         self.operator = operator
@@ -3735,9 +3736,9 @@ class BinaryExpression(ColumnElement):
             self.modifiers = modifiers
 
     def __nonzero__(self):
-        try:
-            return self.operator(hash(self.left), hash(self.right))
-        except:
+        if self.operator in (operator.eq, operator.ne):
+            return self.operator(hash(self._orig[0]), hash(self._orig[1]))
+        else:
             raise TypeError("Boolean value of this clause is not defined")
 
     @property
diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py
index 9da9d94c3..45f4978ed 100644
--- a/test/sql/test_operators.py
+++ b/test/sql/test_operators.py
@@ -833,6 +833,58 @@ class ComparisonOperatorTest(fixtures.TestBase, testing.AssertsCompiledSQL):
     def test_comparison_operators_ge(self):
         self._test_comparison_op(operator.ge, '>=', '<=')
 
+class NonZeroTest(fixtures.TestBase):
+    def _raises(self, expr):
+        assert_raises_message(
+            TypeError,
+            "Boolean value of this clause is not defined",
+            bool, expr
+        )
+
+    def _assert_true(self, expr):
+        is_(bool(expr), True)
+
+    def _assert_false(self, expr):
+        is_(bool(expr), False)
+
+    def test_column_identity_eq(self):
+        c1 = column('c1')
+        self._assert_true(c1 == c1)
+
+    def test_column_identity_gt(self):
+        c1 = column('c1')
+        self._raises(c1 > c1)
+
+    def test_column_compare_eq(self):
+        c1, c2 = column('c1'), column('c2')
+        self._assert_false(c1 == c2)
+
+    def test_column_compare_gt(self):
+        c1, c2 = column('c1'), column('c2')
+        self._raises(c1 > c2)
+
+    def test_binary_identity_eq(self):
+        c1 = column('c1')
+        expr = c1 > 5
+        self._assert_true(expr == expr)
+
+    def test_labeled_binary_identity_eq(self):
+        c1 = column('c1')
+        expr = (c1 > 5).label(None)
+        self._assert_true(expr == expr)
+
+    def test_annotated_binary_identity_eq(self):
+        c1 = column('c1')
+        expr1 = (c1 > 5)
+        expr2 = expr1._annotate({"foo": "bar"})
+        self._assert_true(expr1 == expr2)
+
+    def test_labeled_binary_compare_gt(self):
+        c1 = column('c1')
+        expr1 = (c1 > 5).label(None)
+        expr2 = (c1 > 5).label(None)
+        self._assert_false(expr1 == expr2)
+
 class NegationTest(fixtures.TestBase, testing.AssertsCompiledSQL):
     __dialect__ = 'default'
 
diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index 65dc65470..a60916b44 100644
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -1287,7 +1287,9 @@ class AnnotationsTest(fixtures.TestBase):
             t.c.x,
             a,
             s,
-            s2
+            s2,
+            t.c.x > 1,
+            (t.c.x > 1).label(None)
         ]:
             annot = obj._annotate({})
             eq_(set([obj]), set([annot]))