Skip to content

Commit 39a3105

Browse files
david-rowleydavid-rowley
committed
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
1 parent e2fbb88 commit 39a3105

File tree

6 files changed

+54
-22
lines changed

6 files changed

+54
-22
lines changed

‎src/backend/nodes/copyfuncs.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -2362,7 +2362,8 @@ _copyRestrictInfo(const RestrictInfo *from)
23622362
COPY_SCALAR_FIELD(right_bucketsize);
23632363
COPY_SCALAR_FIELD(left_mcvfreq);
23642364
COPY_SCALAR_FIELD(right_mcvfreq);
2365-
COPY_SCALAR_FIELD(hasheqoperator);
2365+
COPY_SCALAR_FIELD(left_hasheqoperator);
2366+
COPY_SCALAR_FIELD(right_hasheqoperator);
23662367

23672368
return newnode;
23682369
}

‎src/backend/nodes/outfuncs.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -2565,7 +2565,8 @@ _outRestrictInfo(StringInfo str, const RestrictInfo *node)
25652565
WRITE_NODE_FIELD(right_em);
25662566
WRITE_BOOL_FIELD(outer_is_left);
25672567
WRITE_OID_FIELD(hashjoinoperator);
2568-
WRITE_OID_FIELD(hasheqoperator);
2568+
WRITE_OID_FIELD(left_hasheqoperator);
2569+
WRITE_OID_FIELD(right_hasheqoperator);
25692570
}
25702571

25712572
static void

‎src/backend/optimizer/path/joinpath.c

+23-8
Original file line numberDiff line numberDiff line change
@@ -394,27 +394,42 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info,
394394
RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
395395
OpExpr *opexpr;
396396
Node *expr;
397+
Oid hasheqoperator;
397398

398-
/* can't use a memoize node without a valid hash equals operator */
399-
if (!OidIsValid(rinfo->hasheqoperator) ||
399+
opexpr = (OpExpr *) rinfo->clause;
400+
401+
/*
402+
* Bail if the rinfo is not compatible. We need a join OpExpr
403+
* with 2 args.
404+
*/
405+
if (!IsA(opexpr, OpExpr) || list_length(opexpr->args) != 2 ||
400406
!clause_sides_match_join(rinfo, outerrel, innerrel))
401407
{
402408
list_free(*operators);
403409
list_free(*param_exprs);
404410
return false;
405411
}
406412

407-
/*
408-
* We already checked that this is an OpExpr with 2 args when
409-
* setting hasheqoperator.
410-
*/
411-
opexpr = (OpExpr *) rinfo->clause;
412413
if (rinfo->outer_is_left)
414+
{
413415
expr = (Node *) linitial(opexpr->args);
416+
hasheqoperator = rinfo->left_hasheqoperator;
417+
}
414418
else
419+
{
415420
expr = (Node *) lsecond(opexpr->args);
421+
hasheqoperator = rinfo->right_hasheqoperator;
422+
}
423+
424+
/* can't do memoize if we can't hash the outer type */
425+
if (!OidIsValid(hasheqoperator))
426+
{
427+
list_free(*operators);
428+
list_free(*param_exprs);
429+
return false;
430+
}
416431

417-
*operators = lappend_oid(*operators, rinfo->hasheqoperator);
432+
*operators = lappend_oid(*operators, hasheqoperator);
418433
*param_exprs = lappend(*param_exprs, expr);
419434
}
420435
}

‎src/backend/optimizer/plan/initsplan.c

+20-8
Original file line numberDiff line numberDiff line change
@@ -2711,15 +2711,16 @@ check_hashjoinable(RestrictInfo *restrictinfo)
27112711
/*
27122712
* check_memoizable
27132713
* If the restrictinfo's clause is suitable to be used for a Memoize node,
2714-
* set the hasheqoperator to the hash equality operator that will be needed
2715-
* during caching.
2714+
* set the lefthasheqoperator and righthasheqoperator to the hash equality
2715+
* operator that will be needed during caching.
27162716
*/
27172717
static void
27182718
check_memoizable(RestrictInfo *restrictinfo)
27192719
{
27202720
TypeCacheEntry *typentry;
27212721
Expr *clause = restrictinfo->clause;
2722-
Node *leftarg;
2722+
Oid lefttype;
2723+
Oid righttype;
27232724

27242725
if (restrictinfo->pseudoconstant)
27252726
return;
@@ -2728,13 +2729,24 @@ check_memoizable(RestrictInfo *restrictinfo)
27282729
if (list_length(((OpExpr *) clause)->args) != 2)
27292730
return;
27302731

2731-
leftarg = linitial(((OpExpr *) clause)->args);
2732+
lefttype = exprType(linitial(((OpExpr *) clause)->args));
27322733

2733-
typentry = lookup_type_cache(exprType(leftarg), TYPECACHE_HASH_PROC |
2734+
typentry = lookup_type_cache(lefttype, TYPECACHE_HASH_PROC |
27342735
TYPECACHE_EQ_OPR);
27352736

2736-
if (!OidIsValid(typentry->hash_proc) || !OidIsValid(typentry->eq_opr))
2737-
return;
2737+
if (OidIsValid(typentry->hash_proc) && OidIsValid(typentry->eq_opr))
2738+
restrictinfo->left_hasheqoperator = typentry->eq_opr;
2739+
2740+
righttype = exprType(lsecond(((OpExpr *) clause)->args));
2741+
2742+
/*
2743+
* Lookup the right type, unless it's the same as the left type, in which
2744+
* case typentry is already pointing to the required TypeCacheEntry.
2745+
*/
2746+
if (lefttype != righttype)
2747+
typentry = lookup_type_cache(righttype, TYPECACHE_HASH_PROC |
2748+
TYPECACHE_EQ_OPR);
27382749

2739-
restrictinfo->hasheqoperator = typentry->eq_opr;
2750+
if (OidIsValid(typentry->hash_proc) && OidIsValid(typentry->eq_opr))
2751+
restrictinfo->right_hasheqoperator = typentry->eq_opr;
27402752
}

‎src/backend/optimizer/util/restrictinfo.c

+4-2
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,8 @@ make_restrictinfo_internal(PlannerInfo *root,
217217
restrictinfo->left_mcvfreq = -1;
218218
restrictinfo->right_mcvfreq = -1;
219219

220-
restrictinfo->hasheqoperator = InvalidOid;
220+
restrictinfo->left_hasheqoperator = InvalidOid;
221+
restrictinfo->right_hasheqoperator = InvalidOid;
221222

222223
return restrictinfo;
223224
}
@@ -368,7 +369,8 @@ commute_restrictinfo(RestrictInfo *rinfo, Oid comm_op)
368369
result->right_bucketsize = rinfo->left_bucketsize;
369370
result->left_mcvfreq = rinfo->right_mcvfreq;
370371
result->right_mcvfreq = rinfo->left_mcvfreq;
371-
result->hasheqoperator = InvalidOid;
372+
result->left_hasheqoperator = InvalidOid;
373+
result->right_hasheqoperator = InvalidOid;
372374

373375
return result;
374376
}

‎src/include/nodes/pathnodes.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -2122,8 +2122,9 @@ typedef struct RestrictInfo
21222122
Selectivity left_mcvfreq; /* left side's most common val's freq */
21232123
Selectivity right_mcvfreq; /* right side's most common val's freq */
21242124

2125-
/* hash equality operator used for memoize nodes, else InvalidOid */
2126-
Oid hasheqoperator;
2125+
/* hash equality operators used for memoize nodes, else InvalidOid */
2126+
Oid left_hasheqoperator;
2127+
Oid right_hasheqoperator;
21272128
} RestrictInfo;
21282129

21292130
/*

0 commit comments

Comments
 (0)