implement content hashing for custom_op, not identity

Fixed critical SQL caching issue where use of the :meth:`_sql.Operators.op`
custom operator function would not produce an appropriate cache key,
leading to reduce the effectiveness of the SQL cache.

Fixes: #9506
Change-Id: I3eab1ddb5e09a811ad717161a59df0884cdf70ed
(cherry picked from commit 0a0c7c73729152b7606509b6e750371106dfdd46)

diff --git a/doc/build/changelog/unreleased_14/9506.rst b/doc/build/changelog/unreleased_14/9506.rst
new file mode 100644 (file)
index 0000000..2533a98
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/9506.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 9506
+
+    Fixed critical SQL caching issue where use of the
+    :meth:`_sql.Operators.op` custom operator function would not produce an appropriate
+    cache key, leading to reduce the effectiveness of the SQL cache.
+

diff --git a/lib/sqlalchemy/sql/operators.py b/lib/sqlalchemy/sql/operators.py
index 8fd851d1561fabae1a04199870fe7de36f487942..2ce1add26f81791acc2da0436cfdea27d66b21e3 100644 (file)
--- a/lib/sqlalchemy/sql/operators.py
+++ b/lib/sqlalchemy/sql/operators.py
@@ -293,10 +293,24 @@ class custom_op(object):
         )
 
     def __eq__(self, other):
-        return isinstance(other, custom_op) and other.opstring == self.opstring
+        return (
+            isinstance(other, custom_op)
+            and other._hash_key() == self._hash_key()
+        )
 
     def __hash__(self):
-        return id(self)
+        return hash(self._hash_key())
+
+    def _hash_key(self):
+        return (
+            self.__class__,
+            self.opstring,
+            self.precedence,
+            self.is_comparison,
+            self.natural_self_precedent,
+            self.eager_grouping,
+            self.return_type._static_cache_key if self.return_type else None,
+        )
 
     def __call__(self, left, right, **kw):
         return left.operate(self, right, **kw)

diff --git a/lib/sqlalchemy/sql/traversals.py b/lib/sqlalchemy/sql/traversals.py
index 21aa17a0a640d1fbb6ff8e23e406141c6dd44228..de97b9de94c5a69a13c3788bf5efae96e317107b 100644 (file)
--- a/lib/sqlalchemy/sql/traversals.py
+++ b/lib/sqlalchemy/sql/traversals.py
@@ -1300,7 +1300,7 @@ class TraversalComparatorStrategy(InternalTraversal, util.MemoizedSlots):
     def visit_operator(
         self, attrname, left_parent, left, right_parent, right, **kw
     ):
-        return left is right
+        return left == right
 
     def visit_type(
         self, attrname, left_parent, left, right_parent, right, **kw

diff --git a/test/sql/test_compare.py b/test/sql/test_compare.py
index 6cee271c9c12b3e752fa9508157bf44b14d6f57c..c8e1efbf1b7ca8e5e8f00acf354679aa6af51fe5 100644 (file)
--- a/test/sql/test_compare.py
+++ b/test/sql/test_compare.py
@@ -13,6 +13,7 @@ from sqlalchemy import exists
 from sqlalchemy import extract
 from sqlalchemy import Float
 from sqlalchemy import Integer
+from sqlalchemy import literal
 from sqlalchemy import literal_column
 from sqlalchemy import MetaData
 from sqlalchemy import or_
@@ -204,11 +205,23 @@ class CoreFixtures(object):
                 bindparam("bar", type_=String)
             ),
         ),
+        lambda: (
+            literal(1).op("+")(literal(1)),
+            literal(1).op("-")(literal(1)),
+            column("q").op("-")(literal(1)),
+            UnaryExpression(table_a.c.b, modifier=operators.neg),
+            UnaryExpression(table_a.c.b, modifier=operators.desc_op),
+            UnaryExpression(table_a.c.b, modifier=operators.custom_op("!")),
+            UnaryExpression(table_a.c.b, modifier=operators.custom_op("~")),
+        ),
         lambda: (
             column("q") == column("x"),
             column("q") == column("y"),
             column("z") == column("x"),
             column("z") + column("x"),
+            column("z").op("foo")(column("x")),
+            column("z").op("foo")(literal(1)),
+            column("z").op("bar")(column("x")),
             column("z") - column("x"),
             column("x") - column("z"),
             column("z") > column("x"),