Subject: Re: Missing docs on AT TIME ZONE precedence?

From: Andrew Dunstan <[email-masked]>
Date: Mon, 27 Nov 2023 16:09:10 -0500

On 2023-11-27 Mo 15:34, Tom Lane wrote:
> Alvaro Herrera <[email-masked]> writes:
>> (TBH I don't think the added comments really explain the problems fully.
>> That's most likely because I don't actually understand what the problems
>> are.)
> The actual problem is that nobody has applied a cluestick to the SQL
> committee about writing an unambiguous grammar :-(.  But I digress.
>
> I don't like the existing coding for more reasons than just
> underdocumentation.  Global assignment of precedence is a really,
> really dangerous tool for solving ambiguous-grammar problems, because
> it can mask problems unrelated to the one you think you are solving:
> basically, it eliminates bison's complaints about grammar ambiguities
> related to the token you mark.  (Commits 12b716457 and 28a61fc6c are
> relevant here.)  Attaching precedence to individual productions is
> far safer, because it won't have any effect that extends beyond that
> production.  You still need a precedence attached to the lookahead
> token; but I think we should try very hard to not assign a precedence
> different from IDENT's to any unreserved keywords.
>
> After a bit of fooling around I found a patch that seems to meet
> that criterion; attached.
>
> 			

Looks good. Perhaps the comments above the UNBOUNDED precedence setting 
(esp. the first paragraph) need strengthening, with a stern injunction 
to avoid different precedence for non-reserved keywords if at all possible.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

---

From: Tom Lane <[email-masked]>
Date: Tue, 28 Nov 2023 10:27:26 -0500

> to avoid different precedence for non-reserved keywords if at all possible.

OK.  How about rewriting that first para like this?

 * Sometimes it is necessary to assign precedence to keywords that are not
 * really part of the operator hierarchy, in order to resolve grammar
 * ambiguities.  It's best to avoid doing so whenever possible, because such
 * assignments have global effect and may hide ambiguities besides the one
 * you intended to solve.  (Attaching a precedence to a single rule with
 * %prec is far safer and should be preferred.)  If you must give precedence
 * to a new keyword, try very hard to give it the same precedence as IDENT.
 * If the keyword has IDENT's precedence then it clearly acts the same as
 * non-keywords and other similar keywords, thus reducing the risk of
 * unexpected precedence effects.
 * 
 * We used to need to assign IDENT an explicit precedence just less than Op,
 * to support target_el without AS.  While that's not really necessary since
 * we removed postfix operators, we continue to do so because it provides a
 * reference point for a precedence level that we can assign to other
 * keywords that lack a natural precedence level.

			regards, tom lane

---

From: Andrew Dunstan <[email-masked]>
Date: Tue, 28 Nov 2023 10:48:00 -0500

>   * keywords that lack a natural precedence level.

LGTM. Thanks.

cheers

andrew

---

From: Tom Lane <[email-masked]>
Date: Tue, 28 Nov 2023 13:34:38 -0500

> LGTM. Thanks.

Thanks for reviewing.  While checking things over one more time,
I noticed that there was an additional violation of this precept,
dating back to long before we understood the hazards: SET is
given its own priority, when it could perfectly well share that
of IDENT.  I adjusted that and pushed.

			regards, tom lane