From: | Peter Geoghegan <pg(at)heroku(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(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 19:30:02 |
Message-ID: | CAM3SWZS8UoP1Bhy7NHHKVMwFGzSKQeLf3W+y7pMoX5Mn43fw6g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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).
--
Peter Geoghegan
Attachment | Content-Type | Size |
---|---|---|
0002-Avoid-copying-within-tuplesort_gettupleslot.patch | text/x-patch | 6.4 KB |
0001-Remove-should_free-tuplesort-routine-arguments.patch | text/x-patch | 12.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2016-12-12 19:32:57 | Re: pgsql: Add support for temporary replication slots |
Previous Message | Kevin Grittner | 2016-12-12 19:06:23 | Re: [OSSTEST PATCH 0/1] PostgreSQL db: Retry on constraint violation |