Re: Memory leak in incremental sort re-scan

On Thu, Jun 15, 2023 at 6:35 PM Tomas Vondra
<tomas.vondra@enterprisedb.com> wrote:
>
>
>
> On 6/15/23 22:36, Tom Lane wrote:
> > Tomas Vondra <tomas.vondra@enterprisedb.com> writes:
> >> On 6/15/23 22:11, Tom Lane wrote:
> >>> I see zero leakage in that example after applying the attached quick
> >>> hack.  (It might be better to make the check in the caller, or to just
> >>> move the call to ExecInitIncrementalSort.)
> >
> >> Thanks for looking. Are you planning to work on this and push the fix,
> >> or do you want me to finish this up?
> >
> > I'm happy to let you take it -- got lots of other stuff on my plate.
> >
>
> OK, will do.

I think the attached is enough to fix it -- rather than nulling out
the sort states in rescan, we can reset them (as the comment says),
but not set them to null (we also have the same mistake with
presorted_keys). That avoids unnecessary recreation of the sort
states, but it also fixes the problem Tom noted as well: the call to
preparePresortedCols() is already guarded by a test on fullsort_state
being NULL, so with this change we also won't unnecessarily redo that
work.

Regards,
James Coleman