Re: [PATCH] Incremental sort (was: PoC: Partial sort)

On Tue, Mar 31, 2020 at 1:04 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> James Coleman <jtc331(at)gmail(dot)com> writes:
> > + * TuplesortMethod is used in a bitmask in Increment Sort's shared memory
> > + * instrumentation so needs to have each value be a separate bit.
>
> >> I don't quite understand why you skipped "1". (Also, is the use of zero
> >> a wise choice?)
>
> > The assignment of 0 was already there, and there wasn't a comment to
> > indicate why. That ends up meaning we wouldn't display "still in
> > progress" as a type here, which is maybe desirable, but I'm honestly
> > not sure why it was that way originally. I'm curious if you have any
> > thoughts on it.
>
> As things stood, the "= 0" was a no-op, since the first enum value
> would've been that anyway. But if you're converting this set of symbols
> to bits that can be OR'd together, it seems pretty strange to use zero,
> because that can't be distinguished from "absence of any entry".
>
> Perhaps the semantics are such that that's actually sensible, but it's
> far from a straightforward remapping of the old enum.

Right, I didn't see the explicit "= 0" in other enums there, so it
made me wonder if it was intentional to designate that one had to be
0, but I guess without a comment that's a lot of inference.

The semantics seemed somewhat useful here in theory, but since I'm not
hearing a "yeah, that was intentional but not commented", I'm just
going to change it to what you'd naturally expect.

James