On Thu, Jun 15, 2023 at 6:35 PM Tomas Vondra <tomas.von...@enterprisedb.com> wrote: > > > > On 6/15/23 22:36, Tom Lane wrote: > > Tomas Vondra <tomas.von...@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
v2-0001-Fix-memory-leak-in-incremental-sort-rescan.patch
Description: Binary data
