diff options
Diffstat (limited to 'src/backend/executor/execExprInterp.c')
| -rw-r--r-- | src/backend/executor/execExprInterp.c | 21 |
1 files changed, 21 insertions, 0 deletions
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 66a67c72b29..fdb6ada6f31 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -1702,6 +1702,15 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull) * 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 pergroup->transValue without + * regard for either being NULL, because ExecAggTransReparent() + * takes 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 newValue, 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 (DatumGetPointer(newVal) != DatumGetPointer(pergroup->transValue)) newVal = ExecAggTransReparent(aggstate, pertrans, @@ -4087,6 +4096,8 @@ ExecAggTransReparent(AggState *aggstate, AggStatePerTrans pertrans, Datum newValue, bool newValueIsNull, Datum oldValue, bool oldValueIsNull) { + Assert(newValue != oldValue); + if (!newValueIsNull) { MemoryContextSwitchTo(aggstate->curaggcontext->ecxt_per_tuple_memory); @@ -4100,6 +4111,16 @@ ExecAggTransReparent(AggState *aggstate, AggStatePerTrans pertrans, pertrans->transtypeByVal, pertrans->transtypeLen); } + else + { + /* + * Ensure that AggStatePerGroup->transValue ends up being 0, so + * callers can safely compare newValue/oldValue without having to + * check their respective nullness. + */ + newValue = (Datum) 0; + } + if (!oldValueIsNull) { if (DatumIsReadWriteExpandedObject(oldValue, |