Uriy Zhuravlev <u.zhuravlev@postgrespro.ru> writes:
>> I'm dubious that the parsetree representation is well-chosen.
>> Probably a single is_slice flag would have been better.
> What do you mean? This flag is for what? You are about the A_Indices
> node(lidx_default/uidx_default)?
Yes. Those flags are partially redundant with the subtree pointers being
NULL, and there are combinations that would be invalid (such as
lidx_default being set but lidx not being null), and it's pretty unobvious
what the difference in representation is between a non-slice case and a
slice case with only the upper index provided. In fact, since you have
five syntaxes to represent, it's impossible for the two bools to
distinguish them all, which means that at least one case *must* be
identified by null-ness of a pointer contradicting what the corresponding
bool's setting would imply. So this just seems like a mess to me.
I think it would come out cleaner if you had just one bool is_slice,
which corresponds directly to whether a colon was present. The four
sub-possibilities of colon notation would be represented by combinations
of null and non-null lidx and uidx. With is_slice false, the only valid
case is lidx==NULL, uidx!=NULL, as before for non-slice notation.
regards, tom lane