Fix incorrect hash equality operator bug in Memoize

In v14, because we don't have a field in RestrictInfo to cache both the
left and right type's hash equality operator, we just restrict the scope
of Memoize to only when the left and right types of a RestrictInfo are the
same.

In master we add another field to RestrictInfo and cache both hash
equality operators.

Reported-by: Jaime Casanova
Author: David Rowley
Discussion: https://postgr.es/m/20210929185544.GB24346%40ahch-to
Backpatch-through: 14

diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 82464c988966c45ca74b5f5708d43f015c5e124d..ad1ea2ff2f833b3ad061c7edcbff9a27ce02f47c 100644 (file)
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2362,7 +2362,8 @@ _copyRestrictInfo(const RestrictInfo *from)
        COPY_SCALAR_FIELD(right_bucketsize);
        COPY_SCALAR_FIELD(left_mcvfreq);
        COPY_SCALAR_FIELD(right_mcvfreq);
-       COPY_SCALAR_FIELD(hasheqoperator);
+       COPY_SCALAR_FIELD(left_hasheqoperator);
+       COPY_SCALAR_FIELD(right_hasheqoperator);
 
        return newnode;
 }

diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 2e5ed77e1897916d207b3bc25831ae668aafac46..23f23f11dc79d8bc0464f0156c811b81aeb23c4a 100644 (file)
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2565,7 +2565,8 @@ _outRestrictInfo(StringInfo str, const RestrictInfo *node)
        WRITE_NODE_FIELD(right_em);
        WRITE_BOOL_FIELD(outer_is_left);
        WRITE_OID_FIELD(hashjoinoperator);
-       WRITE_OID_FIELD(hasheqoperator);
+       WRITE_OID_FIELD(left_hasheqoperator);
+       WRITE_OID_FIELD(right_hasheqoperator);
 }
 
 static void

diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c
index 6407ede12a612ac188c5aa240e1a549102f5adce..0f3ad8aa658b067665a0075a966da867ac80b4b3 100644 (file)
--- a/src/backend/optimizer/path/joinpath.c
+++ b/src/backend/optimizer/path/joinpath.c
@@ -394,9 +394,15 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info,
                        RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
                        OpExpr     *opexpr;
                        Node       *expr;
+                       Oid                     hasheqoperator;
 
-                       /* can't use a memoize node without a valid hash equals operator */
-                       if (!OidIsValid(rinfo->hasheqoperator) ||
+                       opexpr = (OpExpr *) rinfo->clause;
+
+                       /*
+                        * Bail if the rinfo is not compatible.  We need a join OpExpr
+                        * with 2 args.
+                        */
+                       if (!IsA(opexpr, OpExpr) || list_length(opexpr->args) != 2 ||
                                !clause_sides_match_join(rinfo, outerrel, innerrel))
                        {
                                list_free(*operators);
@@ -404,17 +410,26 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info,
                                return false;
                        }
 
-                       /*
-                        * We already checked that this is an OpExpr with 2 args when
-                        * setting hasheqoperator.
-                        */
-                       opexpr = (OpExpr *) rinfo->clause;
                        if (rinfo->outer_is_left)
+                       {
                                expr = (Node *) linitial(opexpr->args);
+                               hasheqoperator = rinfo->left_hasheqoperator;
+                       }
                        else
+                       {
                                expr = (Node *) lsecond(opexpr->args);
+                               hasheqoperator = rinfo->right_hasheqoperator;
+                       }
+
+                       /* can't do memoize if we can't hash the outer type */
+                       if (!OidIsValid(hasheqoperator))
+                       {
+                               list_free(*operators);
+                               list_free(*param_exprs);
+                               return false;
+                       }
 
-                       *operators = lappend_oid(*operators, rinfo->hasheqoperator);
+                       *operators = lappend_oid(*operators, hasheqoperator);
                        *param_exprs = lappend(*param_exprs, expr);
                }
        }

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index e25dc9a7ca7c25acfedbaf70d2ccb09ff48f6f68..f6a202d900fedb64246da8d3751b76aa33c542f3 100644 (file)
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -2711,15 +2711,16 @@ check_hashjoinable(RestrictInfo *restrictinfo)
 /*
  * check_memoizable
  *       If the restrictinfo's clause is suitable to be used for a Memoize node,
- *       set the hasheqoperator to the hash equality operator that will be needed
- *       during caching.
+ *       set the lefthasheqoperator and righthasheqoperator to the hash equality
+ *       operator that will be needed during caching.
  */
 static void
 check_memoizable(RestrictInfo *restrictinfo)
 {
        TypeCacheEntry *typentry;
        Expr       *clause = restrictinfo->clause;
-       Node       *leftarg;
+       Oid                     lefttype;
+       Oid                     righttype;
 
        if (restrictinfo->pseudoconstant)
                return;
@@ -2728,13 +2729,24 @@ check_memoizable(RestrictInfo *restrictinfo)
        if (list_length(((OpExpr *) clause)->args) != 2)
                return;
 
-       leftarg = linitial(((OpExpr *) clause)->args);
+       lefttype = exprType(linitial(((OpExpr *) clause)->args));
 
-       typentry = lookup_type_cache(exprType(leftarg), TYPECACHE_HASH_PROC |
+       typentry = lookup_type_cache(lefttype, TYPECACHE_HASH_PROC |
                                                                 TYPECACHE_EQ_OPR);
 
-       if (!OidIsValid(typentry->hash_proc) || !OidIsValid(typentry->eq_opr))
-               return;
+       if (OidIsValid(typentry->hash_proc) && OidIsValid(typentry->eq_opr))
+               restrictinfo->left_hasheqoperator = typentry->eq_opr;
+
+       righttype = exprType(lsecond(((OpExpr *) clause)->args));
+
+       /*
+        * Lookup the right type, unless it's the same as the left type, in which
+        * case typentry is already pointing to the required TypeCacheEntry.
+        */
+       if (lefttype != righttype)
+               typentry = lookup_type_cache(righttype, TYPECACHE_HASH_PROC |
+                                                                        TYPECACHE_EQ_OPR);
 
-       restrictinfo->hasheqoperator = typentry->eq_opr;
+       if (OidIsValid(typentry->hash_proc) && OidIsValid(typentry->eq_opr))
+               restrictinfo->right_hasheqoperator = typentry->eq_opr;
 }

diff --git a/src/backend/optimizer/util/restrictinfo.c b/src/backend/optimizer/util/restrictinfo.c
index aa9fb3a9fa5e863fb0c0f4ffe307ba82642fa004..ebfcf91826700c9a1259b6c15c9f8ecc6ff03d82 100644 (file)
--- a/src/backend/optimizer/util/restrictinfo.c
+++ b/src/backend/optimizer/util/restrictinfo.c
@@ -217,7 +217,8 @@ make_restrictinfo_internal(PlannerInfo *root,
        restrictinfo->left_mcvfreq = -1;
        restrictinfo->right_mcvfreq = -1;
 
-       restrictinfo->hasheqoperator = InvalidOid;
+       restrictinfo->left_hasheqoperator = InvalidOid;
+       restrictinfo->right_hasheqoperator = InvalidOid;
 
        return restrictinfo;
 }
@@ -368,7 +369,8 @@ commute_restrictinfo(RestrictInfo *rinfo, Oid comm_op)
        result->right_bucketsize = rinfo->left_bucketsize;
        result->left_mcvfreq = rinfo->right_mcvfreq;
        result->right_mcvfreq = rinfo->left_mcvfreq;
-       result->hasheqoperator = InvalidOid;
+       result->left_hasheqoperator = InvalidOid;
+       result->right_hasheqoperator = InvalidOid;
 
        return result;
 }

diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 2a53a6e344b0dfe98ae31bc990317dc7729b0443..186e89905b2bee92e9ec703caf945bcef2ecae09 100644 (file)
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2122,8 +2122,9 @@ typedef struct RestrictInfo
        Selectivity left_mcvfreq;       /* left side's most common val's freq */
        Selectivity right_mcvfreq;      /* right side's most common val's freq */
 
-       /* hash equality operator used for memoize nodes, else InvalidOid */
-       Oid                     hasheqoperator;
+       /* hash equality operators used for memoize nodes, else InvalidOid */
+       Oid                     left_hasheqoperator;
+       Oid                     right_hasheqoperator;
 } RestrictInfo;
 
 /*