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: David Rowley <dgrowleyml@gmail.com>, Ronan Dunklau <ronan.dunklau@aiven.io>
Cc: Önder Kalacı <onderkalaci@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-09-28T18:34:21Z
Lists: pgsql-hackers 
I wrote:
> and bisecting fingers this commit as the guilty party:

> commit 91e9e89dccdfdf4216953d3d8f5515dcdef177fb
> Author: David Rowley <drowley@postgresql.org>
> Date:   Thu Jul 22 14:03:19 2021 +1200

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

After looking at that for a little while, I wonder if we shouldn't
fix this by restricting the Datum-sort path to be used only with
pass-by-value data types.  That'd require only a minor addition
to the new logic in ExecInitSort.

The alternative of inserting a pfree of the old value would complicate
the code nontrivially, I think, and really it would necessitate a
complete performance re-test.  I'm wondering if the claimed speedup
for pass-by-ref types wasn't fictional and based on skipping the
required pfrees.  Besides, if you think this code is hot enough that
you don't want to add a test-and-branch per tuple (a claim I also
doubt, BTW) then you probably don't want to add such overhead into
the pass-by-value case where the speedup is clear.

			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().