On Tue, Apr 7, 2020 at 7:02 PM Tomas Vondra
<tomas.vondra@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
