From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Sat, 18 Mar 2023 15:43:47 +0000 (-0400)
Subject: implement content hashing for custom_op, not identity
X-Git-Tag: rel_1_4_47~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=953f3d9ccad72396f3af81188189795d805ee913;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

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
index 0000000000..2533a986b1
--- /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 8fd851d156..2ce1add26f 100644
--- 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 21aa17a0a6..de97b9de94 100644
--- 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 6cee271c9c..c8e1efbf1b 100644
--- 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"),