Re: factorial function/phase out postfix operators?

> On Jul 18, 2020, at 1:00 AM, John Naylor <john(dot)naylor(at)2ndquadrant(dot)com> wrote:
>
> On Sat, Jul 11, 2020 at 1:14 AM Mark Dilger
> <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>>
>>> Tom and Álvaro discussed upthread:
>>>
>>>> Would it make sense (and possible) to have a keyword category that is
>>>> not disjoint wrt. the others? Maybe that ends up being easier than
>>>> a solution that ends up with six or seven categories.
>>
>> Version 2, attached, follows this design, increasing the number of keywords that can be used as column aliases without the AS keyword up to 411, with only 39 keywords still requiring an explicit preceding AS.
>
> Hi Mark,
>
> This isn't a full review, but I have a few questions/comments:

Thanks for looking!

> By making col-label-ness an orthogonal attribute, do we still need the
> category of non_label_keyword? It seems not.

You are right. The non_label_keyword category has been removed from v3.

> pg_get_keywords() should probably have a column to display ability to
> act as a bare col label. Perhaps a boolean? If so, what do you think
> of using true/false for the new field in kwlist.h as well?

I have broken this into its own patch. I like using a BARE_LABEL / EXPLICIT_LABEL in kwlist.h because it is self-documenting. I don't care about the *exact* strings that we choose for that, but using TRUE/FALSE in kwlist.h makes it harder for a person adding a new keyword to know what to place there. If they guess "FALSE", and also don't know about adding the new keyword to the bare_label_keyword rule in gram.y, then those two mistakes will agree with each other and the person adding the keyword won't likely know they did it wrong. It is simple enough for gen_keywordlist.pl to convert between what we use in kwlist.h and a boolean value for kwlist_d.h, so I did it that way.

> In the bikeshedding department, it seems "implicit" was chosen because
> it was distinct from "bare". I think "bare" as a descriptor should be
> kept throughout for readability's sake. Maybe BareColLabel could be
> "IDENT or bare_label_keyword" for example. Same for the $status var.

The category "bare_label_keyword" is used in v3. As for the $status var, I don't want to name that $bare, as I didn't go with your idea about using a boolean. $status = "BARE_LABEL" vs "EXPLICIT_LABEL" makes sense to me, more than $bare = "BARE_LABEL" vs "EXPLICIT_LABEL" does. "status" is still a bit vague, so more bikeshedding is welcome.

> Likewise, it seems the actual removal of postfix operators should be a
> separate patch.

I broke out the removal of postfix operators into its own patch in the v3 series.

This patch does not attempt to remove pre-existing postfix operators from existing databases, so users upgrading to the new major version who have custom postfix operators will find that pg_upgrade chokes trying to recreate the postfix operator. That's not great, but perhaps there is nothing automated that we could do for them that would be any better.