Re: tuplesort_gettuple_common() and *should_free argument

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Peter Geoghegan <pg(at)heroku(dot)com>
Cc: hlinnaka <hlinnaka(at)iki(dot)fi>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: tuplesort_gettuple_common() and *should_free argument
Date: 2016-12-12 20:59:20
Message-ID: CA+TgmobCC08z9_N9G4MZiB1uyHthWQhM-nJnENnhyPfJaJRApw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Mon, Dec 12, 2016 at 2:30 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Mon, Dec 12, 2016 at 9:31 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> I think this patch might have a bug. In the existing code,
>> tuplesort_gettupleslot sets should_free = true if it isn't already
>> just before calling ExecStoreMinimalTuple((MinimalTuple) stup.tuple,
>> slot, should_free), so it seems that ExecStoreMinimalTuple() will
>> always get "true" as the fourth argument. However the patch changes
>> that line of code like this:
>>
>> + ExecStoreMinimalTuple((MinimalTuple) stup.tuple, slot, false);
>>
>> So the patch seems to have the effect of changing the fourth argument
>> to this call to ExecStoreMinimalTuple() from always-true to
>> always-false. I might be missing something, but my guess is that's
>> not right.
>
> There was a memory leak added by 0001-*, but then fixed by 0002-*. I
> should have done more testing of 0001-* alone. Oops.
>
> Attached revision of 0001-* fixes this. A revised 0002-* is also
> attached, just as a convenience for reviewers (they won't need to
> resolve the conflict themselves).

Committed 0001.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2016-12-12 21:02:59 Re: jacana hung after failing to acquire random number
Previous Message Robert Haas 2016-12-12 20:33:07 Re: Nested Wait Events?