Re: lcr v5 - introduction of InvalidCommandId

Hi,

Thanks for weighin in.

On 2013-09-05 14:21:33 -0400, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > OK. Consider me more of a -0 than a -1. Like I say, I don't really
> > want to block it; I just don't feel comfortable committing it unless a
> > few other people say something like "I don't see a problem with that".
>
> FWIW, I've always thought it was a wart that there wasn't a recognized
> InvalidCommandId value. It was never pressing to fix it before, but
> if LCR needs it, let's do so.

Yes, its a bit anomalous to the other types.

> I definitely *don't* find it cleaner to
> eat up another flag bit to avoid that. We don't have many to spare.

It wouldn't need to be a flag bit in any existing struct, so that's not
a problem.

> Ideally I'd have made InvalidCommandId = 0 and FirstCommandId = 1,
> but I suppose we can't have that without an on-disk compatibility break.

The patch actually does change it exactly that way. My argument for that
being valid is that CommandIds don't play any role outside of their own
transaction. Now, somebody could argue that SELECT cmin, cmax can be
done outside the transaction, but: Those values are already pretty much
meaningless today since cmin/cmax have been merged. They also don't
check whether the field is initialized at all.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services