Re: tuplesort_gettuple_common() and *should_free argument

Peter Geoghegan <pg@heroku.com>

From: Peter Geoghegan <pg@heroku.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: hlinnaka <hlinnaka@iki.fi>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2016-12-12T19:30:02Z
Lists: pgsql-hackers

Attachments

On Mon, Dec 12, 2016 at 9:31 AM, Robert Haas <robertmhaas@gmail.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

Commits

  1. Fix duplicated words in comment.

  2. Remove should_free arguments to tuplesort routines.

  3. Fix use-after-free around DISTINCT transition function calls.