From 8b07f650c22525d8aa5bbc64e5b6d74bfd607328 Mon Sep 17 00:00:00 2001 From: Linden Lance Date: Fri, 9 Oct 2026 01:34:45 +1300 Subject: [PATCH v3 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 sets up the step's run-time state on the first evaluation -- allocating it and building 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_hash_setup(), which stores the state in the step and returns it. The following patch extends that function for non-Const arrays and makes it skip the hash table for arrays too short to be worth hashing; moving the code here keeps that patch from re-indenting it. * 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 | 266 ++++++++++++++------------ src/backend/optimizer/util/clauses.c | 1 - src/include/executor/execExpr.h | 1 - src/include/nodes/primnodes.h | 3 + 5 files changed, 143 insertions(+), 129 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 3f063b79f4d..408a657d6b8 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..8b1b0b79137 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -4235,11 +4235,146 @@ 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); } +/* + * Set up and return the run-time state on the step's first evaluation: build + * the hash table. + */ +static ScalarArrayOpExprHashTable * +saop_hash_setup(ExprEvalStep *op, ExprContext *econtext) +{ + ScalarArrayOpExprHashTable *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; + MemoryContext cxt = econtext->ecxt_per_query_memory; + MemoryContext oldcontext; + ArrayType *arr; + int nitems; + int16 typlen; + bool typbyval; + char typalign; + uint8 typalignby; + bool has_nulls = false; + char *s; + uint8 *bitmap; + int bitmask; + bool hashfound; + Datum result; + bool resultnull; + + arr = DatumGetArrayTypeP(*op->resvalue); + nitems = ArrayGetNItems(ARR_NDIM(arr), ARR_DIMS(arr)); + + get_typlenbyvalalign(ARR_ELEMTYPE(arr), + &typlen, + &typbyval, + &typalign); + typalignby = typalign_to_alignby(typalign); + + oldcontext = MemoryContextSwitchTo(cxt); + + elements_tab = (ScalarArrayOpExprHashTable *) + palloc0(offsetof(ScalarArrayOpExprHashTable, hash_fcinfo_data) + + SizeForFunctionCallInfo(1)); + op->d.hashedscalararrayop.elements_tab = elements_tab; + elements_tab->op = op; + + MemoryContextSwitchTo(oldcontext); + + fmgr_info_cxt(saop->hashfuncid, &elements_tab->hash_finfo, cxt); + 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(cxt, nitems, elements_tab); + + 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; + } + + return elements_tab; +} + /* * Evaluate "scalar op ANY (const array)". * @@ -4259,7 +4394,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; @@ -4279,130 +4414,9 @@ ExecEvalHashedScalarArrayOp(ExprState *state, ExprEvalStep *op, ExprContext *eco return; } - /* Build the hash table on first evaluation */ + /* Set up 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)); - - get_typlenbyvalalign(ARR_ELEMTYPE(arr), - &typlen, - &typbyval, - &typalign); - typalignby = typalign_to_alignby(typalign); - - oldcontext = MemoryContextSwitchTo(econtext->ecxt_per_query_memory); - - elements_tab = (ScalarArrayOpExprHashTable *) - palloc0(offsetof(ScalarArrayOpExprHashTable, hash_fcinfo_data) + - SizeForFunctionCallInfo(1)); - 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; - } - } + elements_tab = saop_hash_setup(op, econtext); /* * When looking up an SQL NULL value with non-strict functions, we defer @@ -4461,7 +4475,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 95134bfeda3..6a54c47bef7 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -2650,7 +2650,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 726c186c7d2..7439c6732a7 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -937,6 +937,9 @@ typedef struct ScalarArrayOpExpr ParseLoc location; } ScalarArrayOpExpr; +/* Minimum array length for which hashing a ScalarArrayOpExpr pays off */ +#define MIN_ARRAY_SIZE_FOR_HASHED_SAOP 9 + /* * BoolExpr - expression node for the basic Boolean operators AND, OR, NOT * -- 2.53.0