On Tue, Apr 7, 2020 at 7:02 PM Tomas Vondra <tomas.von...@2ndquadrant.com> wrote: > > On Mon, Apr 06, 2020 at 11:25:21PM -0500, Justin Pryzby wrote: > >On Mon, Apr 06, 2020 at 09:57:22PM +0200, Tomas Vondra wrote: > >> I've pushed the fist part of this patch series - I've reorganized it a > > > >I scanned through this again post-commit. Find attached some suggestions. > > > > Thanks. The typo fixes seem clear, except for this bit: > > * If we've set up either of the sort states yet, we need to reset them. > * We could end them and null out the pointers, but there's no reason to > * repay the setup cost, and because ???? guard setting up pivot comparator > * state similarly, doing so might actually cause a leak. > > I can't figure out what ???? should be. James, do you recall what this > should be?
Yep, it's ExecIncrementalSort. If you look for the block guarded by `if (fullsort_state == NULL)` you'll see the call to preparePresortedCols(), which sets up the pivot comparator state referenced by this comment. James
