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
This commit is contained in:
David Rowley 2021-11-08 14:40:33 +13:00
parent e2fbb88372
commit 39a3105678
6 changed files with 54 additions and 22 deletions

View File

@ -2362,7 +2362,8 @@ _copyRestrictInfo(const RestrictInfo *from)
COPY_SCALAR_FIELD(right_bucketsize); COPY_SCALAR_FIELD(right_bucketsize);
COPY_SCALAR_FIELD(left_mcvfreq); COPY_SCALAR_FIELD(left_mcvfreq);
COPY_SCALAR_FIELD(right_mcvfreq); COPY_SCALAR_FIELD(right_mcvfreq);
COPY_SCALAR_FIELD(hasheqoperator); COPY_SCALAR_FIELD(left_hasheqoperator);
COPY_SCALAR_FIELD(right_hasheqoperator);
return newnode; return newnode;
} }

View File

@ -2565,7 +2565,8 @@ _outRestrictInfo(StringInfo str, const RestrictInfo *node)
WRITE_NODE_FIELD(right_em); WRITE_NODE_FIELD(right_em);
WRITE_BOOL_FIELD(outer_is_left); WRITE_BOOL_FIELD(outer_is_left);
WRITE_OID_FIELD(hashjoinoperator); WRITE_OID_FIELD(hashjoinoperator);
WRITE_OID_FIELD(hasheqoperator); WRITE_OID_FIELD(left_hasheqoperator);
WRITE_OID_FIELD(right_hasheqoperator);
} }
static void static void

View File

@ -394,9 +394,15 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info,
RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc);
OpExpr *opexpr; OpExpr *opexpr;
Node *expr; Node *expr;
Oid hasheqoperator;
/* can't use a memoize node without a valid hash equals operator */ opexpr = (OpExpr *) rinfo->clause;
if (!OidIsValid(rinfo->hasheqoperator) ||
/*
* 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)) !clause_sides_match_join(rinfo, outerrel, innerrel))
{ {
list_free(*operators); list_free(*operators);
@ -404,17 +410,26 @@ paraminfo_get_equal_hashops(PlannerInfo *root, ParamPathInfo *param_info,
return false; 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) if (rinfo->outer_is_left)
{
expr = (Node *) linitial(opexpr->args); expr = (Node *) linitial(opexpr->args);
hasheqoperator = rinfo->left_hasheqoperator;
}
else else
{
expr = (Node *) lsecond(opexpr->args); expr = (Node *) lsecond(opexpr->args);
hasheqoperator = rinfo->right_hasheqoperator;
}
*operators = lappend_oid(*operators, rinfo->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, hasheqoperator);
*param_exprs = lappend(*param_exprs, expr); *param_exprs = lappend(*param_exprs, expr);
} }
} }

View File

@ -2711,15 +2711,16 @@ check_hashjoinable(RestrictInfo *restrictinfo)
/* /*
* check_memoizable * check_memoizable
* If the restrictinfo's clause is suitable to be used for a Memoize node, * 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 * set the lefthasheqoperator and righthasheqoperator to the hash equality
* during caching. * operator that will be needed during caching.
*/ */
static void static void
check_memoizable(RestrictInfo *restrictinfo) check_memoizable(RestrictInfo *restrictinfo)
{ {
TypeCacheEntry *typentry; TypeCacheEntry *typentry;
Expr *clause = restrictinfo->clause; Expr *clause = restrictinfo->clause;
Node *leftarg; Oid lefttype;
Oid righttype;
if (restrictinfo->pseudoconstant) if (restrictinfo->pseudoconstant)
return; return;
@ -2728,13 +2729,24 @@ check_memoizable(RestrictInfo *restrictinfo)
if (list_length(((OpExpr *) clause)->args) != 2) if (list_length(((OpExpr *) clause)->args) != 2)
return; 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); TYPECACHE_EQ_OPR);
if (!OidIsValid(typentry->hash_proc) || !OidIsValid(typentry->eq_opr)) if (OidIsValid(typentry->hash_proc) && OidIsValid(typentry->eq_opr))
return; restrictinfo->left_hasheqoperator = typentry->eq_opr;
restrictinfo->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);
if (OidIsValid(typentry->hash_proc) && OidIsValid(typentry->eq_opr))
restrictinfo->right_hasheqoperator = typentry->eq_opr;
} }

View File

@ -217,7 +217,8 @@ make_restrictinfo_internal(PlannerInfo *root,
restrictinfo->left_mcvfreq = -1; restrictinfo->left_mcvfreq = -1;
restrictinfo->right_mcvfreq = -1; restrictinfo->right_mcvfreq = -1;
restrictinfo->hasheqoperator = InvalidOid; restrictinfo->left_hasheqoperator = InvalidOid;
restrictinfo->right_hasheqoperator = InvalidOid;
return restrictinfo; return restrictinfo;
} }
@ -368,7 +369,8 @@ commute_restrictinfo(RestrictInfo *rinfo, Oid comm_op)
result->right_bucketsize = rinfo->left_bucketsize; result->right_bucketsize = rinfo->left_bucketsize;
result->left_mcvfreq = rinfo->right_mcvfreq; result->left_mcvfreq = rinfo->right_mcvfreq;
result->right_mcvfreq = rinfo->left_mcvfreq; result->right_mcvfreq = rinfo->left_mcvfreq;
result->hasheqoperator = InvalidOid; result->left_hasheqoperator = InvalidOid;
result->right_hasheqoperator = InvalidOid;
return result; return result;
} }

View File

@ -2122,8 +2122,9 @@ typedef struct RestrictInfo
Selectivity left_mcvfreq; /* left side's most common val's freq */ Selectivity left_mcvfreq; /* left side's most common val's freq */
Selectivity right_mcvfreq; /* right 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 */ /* hash equality operators used for memoize nodes, else InvalidOid */
Oid hasheqoperator; Oid left_hasheqoperator;
Oid right_hasheqoperator;
} RestrictInfo; } RestrictInfo;
/* /*