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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Rowley <dgrowleyml(at)gmail(dot)com>, Ronan Dunklau <ronan(dot)dunklau(at)aiven(dot)io>
Cc: Önder Kalacı <onderkalaci(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: A potential memory leak on Merge Join when Sort node is not below Materialize node
Date: 2022-09-28 18:34:21
Message-ID: 4022883.1664390061@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> and bisecting fingers this commit as the guilty party:

> commit 91e9e89dccdfdf4216953d3d8f5515dcdef177fb
> Author: David Rowley <drowley(at)postgresql(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2022-09-28 18:50:34 Re: predefined role(s) for VACUUM and ANALYZE
Previous Message Bruce Momjian 2022-09-28 17:40:28 Re: Adding CommandID to heap xlog records