Re: factorial function/phase out postfix operators?

> On Aug 27, 2020, at 2:24 AM, John Naylor <john(dot)naylor(at)2ndquadrant(dot)com> wrote:
>
> On Wed, Aug 26, 2020 at 6:57 PM Mark Dilger
> <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
>>
>>
>>
>>> On Aug 26, 2020, at 6:33 AM, John Naylor <john(dot)naylor(at)2ndquadrant(dot)com> wrote:
>>
>>> and since POSTFIXOP is gone
>>> it doesn't seem to matter how low it is. In fact, I found that the
>>> lines with INDENT and UNBOUNDED now work as the lowest precedence
>>> declarations. Maybe that's worth something?
>>>
>>> Following on Peter E.'s example upthread, GENERATED can be removed
>>> from precedence, and I also found the same is true for PRESERVE and
>>> STRIP_P.
>>>
>>> I've attached a patch which applies on top of 0001 to demonstrate
>>> this. There might possibly still be syntax errors for things not
>>> covered in the regression test, but there are no s/r conflicts at
>>> least.
>>
>> I don't have any problem with the changes you made in your patch, but building on your changes I also found that the following cleanup causes no apparent problems:
>>
>> -%nonassoc UNBOUNDED /* ideally should have same precedence as IDENT */
>> -%nonassoc IDENT PARTITION RANGE ROWS GROUPS PRECEDING FOLLOWING CUBE ROLLUP
>> +%nonassoc UNBOUNDED IDENT
>> +%nonassoc PARTITION RANGE ROWS GROUPS PRECEDING FOLLOWING CUBE ROLLUP
>>
>> Which does what the old comment apparently wanted.
>
> This changes the context of the comment at the top of the block:
>
> * To support target_el without AS, we must give IDENT an explicit priority
> * lower than Op. We can safely assign the same priority to various
> * unreserved keywords as needed to resolve ambiguities (this can't have any
>
> This also works:
>
> -%nonassoc UNBOUNDED /* ideally should have same
> precedence as IDENT */
> -%nonassoc IDENT PARTITION RANGE ROWS GROUPS PRECEDING FOLLOWING
> CUBE ROLLUP
> +%nonassoc UNBOUNDED IDENT PARTITION RANGE ROWS GROUPS CUBE ROLLUP
> +%nonassoc PRECEDING FOLLOWING
>
> Not sure if either is better. Some additional input would be good here.

You wrote in a later email:

> Thinking about this some more, I don't think we don't need to do any
> precedence refactoring in order to apply the functional change of
> these patches. We could leave that for follow-on patches once we
> figure out the best way forward, which could take some time.

So I tried to leave the precedence stuff alone as much as possible in this next patch set. I agree such refactoring can be done separately, and at a later time.

>
> While looking for a place to put a v13 deprecation notice, I found
> some more places in the docs which need updating:
>
> ref/create_operator.sgml
>
> "At least one of LEFTARG and RIGHTARG must be defined. For binary
> operators, both must be defined. For right unary operators, only
> LEFTARG should be defined, while for left unary operators only
> RIGHTARG should be defined."
>
> ref/create_opclass.sgml
>
> "In an OPERATOR clause, the operand data type(s) of the operator, or
> NONE to signify a left-unary or right-unary operator."

Some changes were made on another thread [1] for the deprecation notices, committed recently by Tom, and I think this patch set is compatible with what was done there. This patch set is intended for commit against master, targeted for PostgreSQL 14, so the deprecation notices are removed along with the things that were deprecated. The references to right-unary operators that you call out, above, have been removed.