On Thu, 29 Sept 2022 at 08:57, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> David Rowley <dgrowleyml@gmail.com> writes:
> > I'm wondering if the best way to fix it if doing it that way would be
> > to invent tuplesort_getdatum_nocopy() which would be the same as
> > tuplesort_getdatum() except it wouldn't do the datumCopy for byref
> > types.
>
> Yeah, perhaps. We'd need a clear spec on how long the Datum could
> be presumed good --- probably till the next tuplesort_getdatum_nocopy
> call, but that'd need to be checked --- and then check if that is
> satisfactory for nodeSort's purposes.
Yeah, I think the same rules around scope apply as
tuplesort_gettupleslot() with copy==false. We could do it by adding a
copy flag to the existing function, but I'd rather not add the
branching to that function. It's probably just better to duplicate it
and adjust.
> If we had such a thing, I wonder if any of the other existing
> tuplesort_getdatum callers would be happier with that. nodeAgg for
> one is tediously freeing the result, but could we drop that logic?
Looking at process_ordered_aggregate_single(), it's likely more
efficient to use the nocopy version and just perform a datumCopy()
when we need to store the oldVal. At least, that would be more
efficient when many values are being skipped due to being the same as
the last one.
I've just pushed the disable byref Datums patch I posted earlier. I
only made a small adjustment to make use of the TupleDescAttr() macro.
Önder, thank you for the report.
David