Horton

Re: A potential memory leak on Merge Join when Sort node is not below Materialize node

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Ronan Dunklau <ronan.dunklau@aiven.io>
Cc: David Rowley <dgrowleyml@gmail.com>, Önder Kalacı <onderkalaci@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-09-29T14:10:03Z
Lists: pgsql-hackers 
Ronan Dunklau <ronan.dunklau@aiven.io> writes:
>> 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.

> For the record, I tried to see if gcc would optimize the function by 
> generating two different versions when copy is true or false, thus getting rid 
> of the branching while still having only one function to deal with.

TBH, I think this is completely ridiculous over-optimization.
There's exactly zero evidence that a second copy of the function
would improve performance, or do anything but contribute to code
bloat (which does have a distributed performance cost).

			regards, tom lane

Commits

  1. Allow nodeSort to perform Datum sorts for byref types

  2. Restrict Datum sort optimization to byval types only

  3. Make nodeSort.c use Datum sorts for single column sorts

  4. Fix actual and potential double-frees around tuplesort usage.

  5. Allow avoiding tuple copy within tuplesort_gettupleslot().