From ec08fd4903711a8720feb509ee502de949f92ba0 Mon Sep 17 00:00:00 2001 From: Linden Lance Date: Fri, 9 Oct 2026 01:34:45 +1300 Subject: [PATCH v2 1/2] Refactor hashed ScalarArrayOpExpr evaluation No behavior change. This prepares for a following patch that allows a ScalarArrayOpExpr to be hashed when its array is not a Const but is fixed for the duration of one execution. * The finfo field of the EEOP_HASHED_SCALARARRAYOP step is redundant: it always equals fcinfo_data->flinfo. Use that instead and drop the field. The following patch needs the space for a new field, so ExprEvalStep does not grow. * Move the code that builds the hash table from the array, together with the bookkeeping for NULL elements and for non-strict operators, from ExecEvalHashedScalarArrayOp() into a new function, saop_build_hashtable(). The following patch calls it only for arrays long enough to be worth hashing; moving it here keeps that patch from re-indenting the code. * Move MIN_ARRAY_SIZE_FOR_HASHED_SAOP from clauses.c to primnodes.h, next to ScalarArrayOpExpr, so that the executor can use it too. Discussion: https://postgr.es/m/TY6PR01MB17327691C30B403369B16B02AF4B02@TY6PR01MB17327.jpnprd01.prod.outlook.com --- src/backend/executor/execExpr.c | 1 - src/backend/executor/execExprInterp.c | 222 ++++++++++++++------------ src/backend/optimizer/util/clauses.c | 1 - src/include/executor/execExpr.h | 1 - src/include/nodes/primnodes.h | 6 + 5 files changed, 130 insertions(+), 101 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 82e846a1f4f..10cdac20de6 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1344,7 +1344,6 @@ ExecInitExprRec(Expr *node, ExprState *state, /* And perform the operation */ scratch.opcode = EEOP_HASHED_SCALARARRAYOP; scratch.d.hashedscalararrayop.inclause = opexpr->useOr; - scratch.d.hashedscalararrayop.finfo = finfo; scratch.d.hashedscalararrayop.fcinfo_data = fcinfo; scratch.d.hashedscalararrayop.saop = opexpr; diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 397219f7a3a..816c2baea23 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -219,6 +219,9 @@ typedef struct ScalarArrayOpExprHashEntry static bool saop_hash_element_match(struct saophash_hash *tb, Datum key1, Datum key2); static uint32 saop_element_hash(struct saophash_hash *tb, Datum key); +static void saop_build_hashtable(ExprEvalStep *op, ExprContext *econtext, + ArrayType *arr, int nitems, int16 typlen, + bool typbyval, char typalign); /* * ScalarArrayOpExprHashTable @@ -4235,11 +4238,127 @@ saop_hash_element_match(struct saophash_hash *tb, Datum key1, Datum key2) fcinfo->args[1].value = key2; fcinfo->args[1].isnull = false; - result = elements_tab->op->d.hashedscalararrayop.finfo->fn_addr(fcinfo); + result = fcinfo->flinfo->fn_addr(fcinfo); return DatumGetBool(result); } +/* + * Build the hash table for an EEOP_HASHED_SCALARARRAYOP step from 'arr', an + * array of 'nitems' elements of the given type, in the step's already + * allocated run-time state. Also remember whether the array has any NULLs + * and, for a non-strict operator, the result of looking up a NULL. + */ +static void +saop_build_hashtable(ExprEvalStep *op, ExprContext *econtext, ArrayType *arr, + int nitems, int16 typlen, bool typbyval, char typalign) +{ + ScalarArrayOpExprHashTable *elements_tab = op->d.hashedscalararrayop.elements_tab; + ScalarArrayOpExpr *saop = op->d.hashedscalararrayop.saop; + FunctionCallInfo fcinfo = op->d.hashedscalararrayop.fcinfo_data; + bool inclause = op->d.hashedscalararrayop.inclause; + bool strictfunc = fcinfo->flinfo->fn_strict; + uint8 typalignby = typalign_to_alignby(typalign); + bool has_nulls = false; + char *s; + uint8 *bitmap; + int bitmask; + bool hashfound; + Datum result; + bool resultnull; + MemoryContext oldcontext; + + oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory); + + fmgr_info(saop->hashfuncid, &elements_tab->hash_finfo); + fmgr_info_set_expr((Node *) saop, &elements_tab->hash_finfo); + + InitFunctionCallInfoData(elements_tab->hash_fcinfo_data, + &elements_tab->hash_finfo, + 1, + saop->inputcollid, + NULL, + NULL); + + /* + * Create the hash table sizing it according to the number of elements in + * the array. This does assume that the array has no duplicates. If the + * array happens to contain many duplicate values then it'll just mean + * that we sized the table a bit on the large side. + */ + elements_tab->hashtab = saophash_create(CurrentMemoryContext, nitems, + elements_tab); + + MemoryContextSwitchTo(oldcontext); + + s = (char *) ARR_DATA_PTR(arr); + bitmap = ARR_NULLBITMAP(arr); + bitmask = 1; + for (int i = 0; i < nitems; i++) + { + /* Get array element, checking for NULL. */ + if (bitmap && (*bitmap & bitmask) == 0) + { + has_nulls = true; + } + else + { + Datum element; + + element = fetch_att(s, typbyval, typlen); + s = att_addlength_pointer(s, typlen, s); + s = (char *) att_nominal_alignby(s, typalignby); + + saophash_insert(elements_tab->hashtab, element, &hashfound); + } + + /* Advance bitmap pointer if any. */ + if (bitmap) + { + bitmask <<= 1; + if (bitmask == 0x100) + { + bitmap++; + bitmask = 1; + } + } + } + + /* + * Remember if we had any nulls so that we know if we need to execute + * non-strict functions with a null lhs value if no match is found. + */ + op->d.hashedscalararrayop.has_nulls = has_nulls; + + /* + * When we have a non-strict equality function, check and cache the result + * from looking up a NULL. Non-strict functions are free to treat a NULL + * as equal to any other value, e.g. a 0 or an empty string. Here we + * perform a linear search over the array and cache the outcome so that we + * can use that result any time we receive a NULL. + */ + if (!strictfunc) + { + bool null_lhs_result; + + fcinfo->args[0].value = (Datum) 0; + fcinfo->args[0].isnull = true; + + ExecEvalArrayCompareInternal(fcinfo, arr, typlen, typbyval, + typalign, true, &result, + &resultnull); + + null_lhs_result = DatumGetBool(result); + + /* invert non-NULL results for NOT IN */ + if (!resultnull && !inclause) + null_lhs_result = !null_lhs_result; + + op->d.hashedscalararrayop.null_lhs_isnull = resultnull; + op->d.hashedscalararrayop.null_lhs_result = null_lhs_result; + } +} + /* * Evaluate "scalar op ANY (const array)". * @@ -4259,7 +4378,7 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco ScalarArrayOpExprHashTable *elements_tab = op->d.hashedscalararrayop.elements_tab; FunctionCallInfo fcinfo = op->d.hashedscalararrayop.fcinfo_data; bool inclause = op->d.hashedscalararrayop.inclause; - bool strictfunc = op->d.hashedscalararrayop.finfo->fn_strict; + bool strictfunc = fcinfo->flinfo->fn_strict; Datum scalar = fcinfo->args[0].value; bool scalar_isnull = fcinfo->args[0].isnull; Datum result; @@ -4282,21 +4401,13 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco /* Build the hash table on first evaluation */ if (elements_tab == NULL) { - ScalarArrayOpExpr *saop; int16 typlen; bool typbyval; char typalign; - uint8 typalignby; int nitems; - bool has_nulls = false; - char *s; - uint8 *bitmap; - int bitmask; MemoryContext oldcontext; ArrayType *arr; - saop = op->d.hashedscalararrayop.saop; - arr = DatumGetArrayTypeP(*op->resvalue); nitems = ArrayGetNItems(ARR_NDIM(arr), ARR_DIMS(arr)); @@ -4304,7 +4415,6 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco &typlen, &typbyval, &typalign); - typalignby = typalign_to_alignby(typalign); oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory); @@ -4314,94 +4424,10 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco op->d.hashedscalararrayop.elements_tab = elements_tab; elements_tab->op = op; - fmgr_info(saop->hashfuncid, &elements_tab->hash_finfo); - fmgr_info_set_expr((Node *) saop, &elements_tab->hash_finfo); - - InitFunctionCallInfoData(elements_tab->hash_fcinfo_data, - &elements_tab->hash_finfo, - 1, - saop->inputcollid, - NULL, - NULL); - - /* - * Create the hash table sizing it according to the number of elements - * in the array. This does assume that the array has no duplicates. - * If the array happens to contain many duplicate values then it'll - * just mean that we sized the table a bit on the large side. - */ - elements_tab->hashtab = saophash_create(CurrentMemoryContext, nitems, - elements_tab); - MemoryContextSwitchTo(oldcontext); - s = (char *) ARR_DATA_PTR(arr); - bitmap = ARR_NULLBITMAP(arr); - bitmask = 1; - for (int i = 0; i < nitems; i++) - { - /* Get array element, checking for NULL. */ - if (bitmap && (*bitmap & bitmask) == 0) - { - has_nulls = true; - } - else - { - Datum element; - - element = fetch_att(s, typbyval, typlen); - s = att_addlength_pointer(s, typlen, s); - s = (char *) att_nominal_alignby(s, typalignby); - - saophash_insert(elements_tab->hashtab, element, &hashfound); - } - - /* Advance bitmap pointer if any. */ - if (bitmap) - { - bitmask <<= 1; - if (bitmask == 0x100) - { - bitmap++; - bitmask = 1; - } - } - } - - /* - * Remember if we had any nulls so that we know if we need to execute - * non-strict functions with a null lhs value if no match is found. - */ - op->d.hashedscalararrayop.has_nulls = has_nulls; - - /* - * When we have a non-strict equality function, check and cache the - * result from looking up a NULL. Non-strict functions are free to - * treat a NULL as equal to any other value, e.g. a 0 or an empty - * string. Here we perform a linear search over the array and cache - * the outcome so that we can use that result any time we receive a - * NULL. - */ - if (!strictfunc) - { - bool null_lhs_result; - - fcinfo->args[0].value = (Datum) 0; - fcinfo->args[0].isnull = true; - - ExecEvalArrayCompareInternal(fcinfo, arr, typlen, typbyval, - typalign, true, &result, - &resultnull); - - null_lhs_result = DatumGetBool(result); - - /* invert non-NULL results for NOT IN */ - if (!resultnull && !inclause) - null_lhs_result = !null_lhs_result; - - op->d.hashedscalararrayop.null_lhs_isnull = resultnull; - op->d.hashedscalararrayop.null_lhs_result = null_lhs_result; - } + saop_build_hashtable(op, econtext, arr, nitems, + typlen, typbyval, typalign); } /* @@ -4461,7 +4487,7 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco fcinfo->args[1].value = (Datum) 0; fcinfo->args[1].isnull = true; - result = op->d.hashedscalararrayop.finfo->fn_addr(fcinfo); + result = fcinfo->flinfo->fn_addr(fcinfo); resultnull = fcinfo->isnull; /* diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 55cebe4a74b..f78f874e5c3 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -2629,7 +2629,6 @@ eval_const_expressions(PlannerInfo *root, Node *node) return eval_const_expressions_mutator(node, &context); } -#define MIN_ARRAY_SIZE_FOR_HASHED_SAOP 9 /*-------------------- * convert_saop_to_hashed_saop * diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index c61b3d624d5..fd1454d685b 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -646,7 +646,6 @@ typedef struct ExprEvalStep * returns. */ bool null_lhs_isnull; struct ScalarArrayOpExprHashTable *elements_tab; - FmgrInfo *finfo; /* function's lookup data */ FunctionCallInfo fcinfo_data; /* arguments etc */ ScalarArrayOpExpr *saop; } hashedscalararrayop; diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 09b0c29408c..948a85b94fb 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -937,6 +937,12 @@ typedef struct ScalarArrayOpExpr ParseLoc location; } ScalarArrayOpExpr; +/* + * Minimum array length for which hashing a ScalarArrayOpExpr beats a linear + * search + */ +#define MIN_ARRAY_SIZE_FOR_HASHED_SAOP 9 + /* * BoolExpr - expression node for the basic Boolean operators AND, OR, NOT * base-commit: a12600b762c36d91450ce085fa25ef75250bc1c2 -- 2.53.0