diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index a49af6bf691..5c20b8a45ab 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -812,6 +812,15 @@ advance_transition_function(AggState *aggstate, * first input, we don't need to do anything. Also, if transfn returned a * pointer to a R/W expanded object that is already a child of the * aggcontext, assume we can adopt that value without copying it. + * + * It's safe to compare newVal with pergroupstate->transValue without + * regard for either being NULL, because we below take care to set + * transValue to 0 when NULL. Otherwise we could end up accidentally not + * reparenting, when the transValue has the same numerical value as + * newVal, despite being NULL. This is a somewhat hot path, making it + * undesirable to instead solve this with another branch for the common + * case of the transition function returning its (modified) input + * argument. */ if (!pertrans->transtypeByVal && DatumGetPointer(newVal) != DatumGetPointer(pergroupstate->transValue)) @@ -829,6 +838,16 @@ advance_transition_function(AggState *aggstate, pertrans->transtypeByVal, pertrans->transtypeLen); } + else + { + /* + * Ensure that pergroupstate->transValue ends up being 0, so we + * can safely compare newVal/transValue without having to check + * their respective nullness. + */ + newVal = (Datum) 0; + } + if (!pergroupstate->transValueIsNull) { if (DatumIsReadWriteExpandedObject(pergroupstate->transValue,