database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-08-25 20:23:07 +03:00
Code Activity

Fix hypothetical bug in ExprState building for hashing

Browse Source 
adf97c156 gave ExprStates the ability to hash expressions and return a
single hash value.  That commit supports seeding the hash value with an
initial value to have that blended into the final hash value.

Here we fix a hypothetical bug where if there are zero expressions to
hash, the initial value is stored in the wrong location.  The existing
code stored the initial value in an intermediate location expecting that
when the expressions were hashed that those steps would store the final
hash value in the ExprState.resvalue field.  However, that wouldn't happen
when there are zero expressions to hash.  The correct thing to do instead
is to have a special case for zero expressions and when we hit that case,
store the initial value directly in the ExprState.resvalue.  The reason
that this is a hypothetical bug is that no code currently calls
ExecBuildHash32Expr passing a non-zero initial value.

Discussion: https://postgr.es/m/CAApHDvpMAL_zxbMRr1LOex3O7Y7R7ZN2i8iUFLQhqQiJMAg3qw@mail.gmail.com
This commit is contained in:
David Rowley
2024-11-06 09:16:00 +13:00
parent 7bdaa4b542
commit 87f81a5563
1 changed files with 14 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
src/backend/executor/execExpr.c
View File

@@ -4004,8 +4004,9 @@ ExecBuildHash32Expr(TupleDesc desc, const TupleTableSlotOps *ops,
ListCell *lc2; ListCell *lc2;
intptr_t strict_opcode; intptr_t strict_opcode;
intptr_t opcode; intptr_t opcode;
int num_exprs = list_length(hash_exprs);
Assert(list_length(hash_exprs) == list_length(collations)); Assert(num_exprs == list_length(collations));
state->parent = parent; state->parent = parent;
@@ -4013,11 +4014,11 @@ ExecBuildHash32Expr(TupleDesc desc, const TupleTableSlotOps *ops,
ExecCreateExprSetupSteps(state, (Node *) hash_exprs); ExecCreateExprSetupSteps(state, (Node *) hash_exprs);
/* /*
* When hashing more than 1 expression or if we have an init value, we * Make a place to store intermediate hash values between subsequent
* need somewhere to store the intermediate hash value so that it's * hashing of individual expressions. We only need this if there is more
* available to be combined with the result of subsequent hashing. * than one expression to hash or an initial value plus one expression.
*/ */
if (list_length(hash_exprs) > 1 || init_value != 0) if ((int64) num_exprs + (init_value != 0) > 1)
iresult = palloc(sizeof(NullableDatum)); iresult = palloc(sizeof(NullableDatum));
if (init_value == 0) if (init_value == 0)
@@ -4032,11 +4033,15 @@ ExecBuildHash32Expr(TupleDesc desc, const TupleTableSlotOps *ops,
} }
else else
{ {
/* Set up operation to set the initial value. */ /*
* Set up operation to set the initial value. Normally we store this
* in the intermediate hash value location, but if there are no exprs
* to hash, store it in the ExprState's result field.
*/
scratch.opcode = EEOP_HASHDATUM_SET_INITVAL; scratch.opcode = EEOP_HASHDATUM_SET_INITVAL;
scratch.d.hashdatum_initvalue.init_value = UInt32GetDatum(init_value); scratch.d.hashdatum_initvalue.init_value = UInt32GetDatum(init_value);
scratch.resvalue = &iresult->value; scratch.resvalue = num_exprs > 0 ? &iresult->value : &state->resvalue;
scratch.resnull = &iresult->isnull; scratch.resnull = num_exprs > 0 ? &iresult->isnull : &state->resnull;
ExprEvalPushStep(state, &scratch); ExprEvalPushStep(state, &scratch);
@@ -4074,7 +4079,7 @@ ExecBuildHash32Expr(TupleDesc desc, const TupleTableSlotOps *ops,
&fcinfo->args[0].value, &fcinfo->args[0].value,
&fcinfo->args[0].isnull); &fcinfo->args[0].isnull);
if (i == list_length(hash_exprs) - 1) if (i == num_exprs - 1)
{ {
/* the result for hashing the final expr is stored in the state */ /* the result for hashing the final expr is stored in the state */
scratch.resvalue = &state->resvalue; scratch.resvalue = &state->resvalue;