diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index a3414a76e8..41f1ca65d0 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -1879,17 +1879,13 @@ heapam_index_validate_scan(Relation heapRelation, } tuplesort_empty = !tuplesort_getdatum(state->tuplesort, true, - &ts_val, &ts_isnull, NULL); + false, &ts_val, &ts_isnull, + NULL); Assert(tuplesort_empty || !ts_isnull); if (!tuplesort_empty) { itemptr_decode(&decoded, DatumGetInt64(ts_val)); indexcursor = &decoded; - - /* If int8 is pass-by-ref, free (encoded) TID Datum memory */ -#ifndef USE_FLOAT8_BYVAL - pfree(DatumGetPointer(ts_val)); -#endif } else { diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index bcf3b1950b..986f761e87 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -879,7 +879,7 @@ process_ordered_aggregate_single(AggState *aggstate, */ while (tuplesort_getdatum(pertrans->sortstates[aggstate->current_set], - true, newVal, isNull, &newAbbrevVal)) + true, false, newVal, isNull, &newAbbrevVal)) { /* * Clear and select the working context for evaluation of the equality @@ -900,24 +900,33 @@ process_ordered_aggregate_single(AggState *aggstate, pertrans->aggCollation, oldVal, *newVal))))) { - /* equal to prior, so forget this one */ - if (!pertrans->inputtypeByVal && !*isNull) - pfree(DatumGetPointer(*newVal)); + MemoryContextSwitchTo(oldContext); + continue; } else { advance_transition_function(aggstate, pertrans, pergroupstate); - /* forget the old value, if any */ - if (!oldIsNull && !pertrans->inputtypeByVal) - pfree(DatumGetPointer(oldVal)); - /* and remember the new one for subsequent equality checks */ - oldVal = *newVal; + + MemoryContextSwitchTo(oldContext); + + /* + * Forget the old value, if any and remember the new one for + * subsequent equality checks. + */ + if (!pertrans->inputtypeByVal) + { + if (!oldIsNull) + pfree(DatumGetPointer(oldVal)); + if (!*isNull) + oldVal = datumCopy(*newVal, pertrans->inputtypeByVal, + pertrans->inputtypeLen); + } + else + oldVal = *newVal; oldAbbrevVal = newAbbrevVal; oldIsNull = *isNull; haveOldVal = true; } - - MemoryContextSwitchTo(oldContext); } if (!oldIsNull && !pertrans->inputtypeByVal) diff --git a/src/backend/executor/nodeSort.c b/src/backend/executor/nodeSort.c index 37ad35704b..a8316f0e13 100644 --- a/src/backend/executor/nodeSort.c +++ b/src/backend/executor/nodeSort.c @@ -198,7 +198,8 @@ ExecSort(PlanState *pstate) { ExecClearTuple(slot); if (tuplesort_getdatum(tuplesortstate, ScanDirectionIsForward(dir), - &(slot->tts_values[0]), &(slot->tts_isnull[0]), NULL)) + false, &(slot->tts_values[0]), + &(slot->tts_isnull[0]), NULL)) ExecStoreVirtualTuple(slot); } else @@ -278,10 +279,10 @@ ExecInitSort(Sort *node, EState *estate, int eflags) outerTupDesc = ExecGetResultType(outerPlanState(sortstate)); /* - * We perform a Datum sort when we're sorting just a single byval column, + * We perform a Datum sort when we're sorting just a single column, * otherwise we perform a tuple sort. */ - if (outerTupDesc->natts == 1 && TupleDescAttr(outerTupDesc, 0)->attbyval) + if (outerTupDesc->natts == 1) sortstate->datumSort = true; else sortstate->datumSort = false; diff --git a/src/backend/utils/adt/orderedsetaggs.c b/src/backend/utils/adt/orderedsetaggs.c index 185b2cb848..d19e7ba871 100644 --- a/src/backend/utils/adt/orderedsetaggs.c +++ b/src/backend/utils/adt/orderedsetaggs.c @@ -482,7 +482,8 @@ percentile_disc_final(PG_FUNCTION_ARGS) elog(ERROR, "missing row in percentile_disc"); } - if (!tuplesort_getdatum(osastate->sortstate, true, &val, &isnull, NULL)) + if (!tuplesort_getdatum(osastate->sortstate, true, true, &val, &isnull, + NULL)) elog(ERROR, "missing row in percentile_disc"); /* We shouldn't have stored any nulls, but do the right thing anyway */ @@ -581,7 +582,8 @@ percentile_cont_final_common(FunctionCallInfo fcinfo, if (!tuplesort_skiptuples(osastate->sortstate, first_row, true)) elog(ERROR, "missing row in percentile_cont"); - if (!tuplesort_getdatum(osastate->sortstate, true, &first_val, &isnull, NULL)) + if (!tuplesort_getdatum(osastate->sortstate, true, true, &first_val, + &isnull, NULL)) elog(ERROR, "missing row in percentile_cont"); if (isnull) PG_RETURN_NULL(); @@ -592,7 +594,8 @@ percentile_cont_final_common(FunctionCallInfo fcinfo, } else { - if (!tuplesort_getdatum(osastate->sortstate, true, &second_val, &isnull, NULL)) + if (!tuplesort_getdatum(osastate->sortstate, true, true, &second_val, + &isnull, NULL)) elog(ERROR, "missing row in percentile_cont"); if (isnull) @@ -817,7 +820,8 @@ percentile_disc_multi_final(PG_FUNCTION_ARGS) if (!tuplesort_skiptuples(osastate->sortstate, target_row - rownum - 1, true)) elog(ERROR, "missing row in percentile_disc"); - if (!tuplesort_getdatum(osastate->sortstate, true, &val, &isnull, NULL)) + if (!tuplesort_getdatum(osastate->sortstate, true, true, &val, + &isnull, NULL)) elog(ERROR, "missing row in percentile_disc"); rownum = target_row; @@ -945,8 +949,8 @@ percentile_cont_multi_final_common(FunctionCallInfo fcinfo, if (!tuplesort_skiptuples(osastate->sortstate, first_row - rownum - 1, true)) elog(ERROR, "missing row in percentile_cont"); - if (!tuplesort_getdatum(osastate->sortstate, true, &first_val, - &isnull, NULL) || isnull) + if (!tuplesort_getdatum(osastate->sortstate, true, true, + &first_val, &isnull, NULL) || isnull) elog(ERROR, "missing row in percentile_cont"); rownum = first_row; @@ -966,8 +970,8 @@ percentile_cont_multi_final_common(FunctionCallInfo fcinfo, /* Fetch second_row if needed */ if (second_row > rownum) { - if (!tuplesort_getdatum(osastate->sortstate, true, &second_val, - &isnull, NULL) || isnull) + if (!tuplesort_getdatum(osastate->sortstate, true, true, + &second_val, &isnull, NULL) || isnull) elog(ERROR, "missing row in percentile_cont"); rownum++; } @@ -1073,7 +1077,8 @@ mode_final(PG_FUNCTION_ARGS) tuplesort_rescan(osastate->sortstate); /* Scan tuples and count frequencies */ - while (tuplesort_getdatum(osastate->sortstate, true, &val, &isnull, &abbrev_val)) + while (tuplesort_getdatum(osastate->sortstate, true, true, &val, &isnull, + &abbrev_val)) { /* we don't expect any nulls, but ignore them if found */ if (isnull) diff --git a/src/backend/utils/sort/tuplesortvariants.c b/src/backend/utils/sort/tuplesortvariants.c index afa5bdbf04..f5d72b4752 100644 --- a/src/backend/utils/sort/tuplesortvariants.c +++ b/src/backend/utils/sort/tuplesortvariants.c @@ -848,9 +848,19 @@ tuplesort_getindextuple(Tuplesortstate *state, bool forward) * determination of "non-equal tuple" based on simple binary inequality. A * NULL value will have a zeroed abbreviated value representation, which caller * may rely on in abbreviated inequality check. + * + * For byref Datums, if copy is true, *val is set to a copy of the Datum + * copied into the caller's memory context, so that it will stay valid + * regardless of future manipulations of the tuplesort's state (up to and + * including deleting the tuplesort). If copy is false, *val will just be + * set to a pointer to the Datum held within the tuplesort, which is more + * efficient, but only safe for callers that are prepared to have any + * subsequent manipulation of the tuplesort's state invalidate slot contents. + * For byval Datums, the value of the 'copy' parameter has no effect. + */ bool -tuplesort_getdatum(Tuplesortstate *state, bool forward, +tuplesort_getdatum(Tuplesortstate *state, bool forward, bool copy, Datum *val, bool *isNull, Datum *abbrev) { TuplesortPublic *base = TuplesortstateGetPublic(state); @@ -879,7 +889,11 @@ tuplesort_getdatum(Tuplesortstate *state, bool forward, else { /* use stup.tuple because stup.datum1 may be an abbreviation */ - *val = datumCopy(PointerGetDatum(stup.tuple), false, arg->datumTypeLen); + if (copy) + *val = datumCopy(PointerGetDatum(stup.tuple), false, + arg->datumTypeLen); + else + *val = PointerGetDatum(stup.tuple); *isNull = false; } diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h index 4441274990..59fbe2bd35 100644 --- a/src/include/utils/tuplesort.h +++ b/src/include/utils/tuplesort.h @@ -439,7 +439,7 @@ extern bool tuplesort_gettupleslot(Tuplesortstate *state, bool forward, bool copy, TupleTableSlot *slot, Datum *abbrev); extern HeapTuple tuplesort_getheaptuple(Tuplesortstate *state, bool forward); extern IndexTuple tuplesort_getindextuple(Tuplesortstate *state, bool forward); -extern bool tuplesort_getdatum(Tuplesortstate *state, bool forward, +extern bool tuplesort_getdatum(Tuplesortstate *state, bool forward, bool copy, Datum *val, bool *isNull, Datum *abbrev);