On Tue, Dec 22, 2015 at 2:28 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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.
Patch is still in the works and author is still active, so moved to next CF.
--
Michael