Re: Missing docs on AT TIME ZONE precedence? - Mailing list pgsql-hackers

Alvaro Herrera <alvherre@alvh.no-ip.org> 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.

            regards, tom lane

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 8c00b119ec..340823588a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -821,11 +821,6 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 %nonassoc    BETWEEN IN_P LIKE ILIKE SIMILAR NOT_LA
 %nonassoc    ESCAPE            /* ESCAPE must be just above LIKE/ILIKE/SIMILAR */

-/* SQL/JSON related keywords */
-%nonassoc    UNIQUE JSON
-%nonassoc    KEYS OBJECT_P SCALAR VALUE_P
-%nonassoc    WITH WITHOUT
-
 /*
  * To support target_el without AS, it used to be necessary to assign IDENT an
  * explicit precedence just less than Op.  While that's not really necessary
@@ -850,9 +845,15 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
  * an explicit priority lower than '(', so that a rule with CUBE '(' will shift
  * rather than reducing a conflicting rule that takes CUBE as a function name.
  * Using the same precedence as IDENT seems right for the reasons given above.
+ *
+ * KEYS, OBJECT_P, SCALAR, VALUE_P, WITH, and WITHOUT are similarly assigned
+ * the same precedence as IDENT.  This allows resolving conflicts in the
+ * json_predicate_type_constraint and json_key_uniqueness_constraint_opt
+ * productions (see comments there).
  */
 %nonassoc    UNBOUNDED        /* ideally would have same precedence as IDENT */
 %nonassoc    IDENT PARTITION RANGE ROWS GROUPS PRECEDING FOLLOWING CUBE ROLLUP
+            KEYS OBJECT_P SCALAR VALUE_P WITH WITHOUT
 %left        Op OPERATOR        /* multi-character ops and user-defined operators */
 %left        '+' '-'
 %left        '*' '/' '%'
@@ -16519,21 +16520,35 @@ json_returning_clause_opt:
             | /* EMPTY */                            { $$ = NULL; }
         ;

+/*
+ * We must assign the only-JSON production a precedence less than IDENT in
+ * order to favor shifting over reduction when JSON is followed by VALUE_P,
+ * OBJECT_P, or SCALAR.  (ARRAY doesn't need that treatment, because it's a
+ * fully reserved word.)  Because json_predicate_type_constraint is always
+ * followed by json_key_uniqueness_constraint_opt, we also need the only-JSON
+ * production to have precedence less than WITH and WITHOUT.  UNBOUNDED isn't
+ * really related to this syntax, but it's a convenient choice because it
+ * already has a precedence less than IDENT for other reasons.
+ */
 json_predicate_type_constraint:
-            JSON                                    { $$ = JS_TYPE_ANY; }
+            JSON                    %prec UNBOUNDED    { $$ = JS_TYPE_ANY; }
             | JSON VALUE_P                            { $$ = JS_TYPE_ANY; }
             | JSON ARRAY                            { $$ = JS_TYPE_ARRAY; }
             | JSON OBJECT_P                            { $$ = JS_TYPE_OBJECT; }
             | JSON SCALAR                            { $$ = JS_TYPE_SCALAR; }
         ;

-/* KEYS is a noise word here */
+/*
+ * KEYS is a noise word here.  To avoid shift/reduce conflicts, assign the
+ * KEYS-less productions a precedence less than IDENT (i.e., less than KEYS).
+ * This prevents reducing them when the next token is KEYS.
+ */
 json_key_uniqueness_constraint_opt:
             WITH UNIQUE KEYS                            { $$ = true; }
-            | WITH UNIQUE                                { $$ = true; }
+            | WITH UNIQUE                %prec UNBOUNDED    { $$ = true; }
             | WITHOUT UNIQUE KEYS                        { $$ = false; }
-            | WITHOUT UNIQUE                            { $$ = false; }
-            | /* EMPTY */                 %prec KEYS        { $$ = false; }
+            | WITHOUT UNIQUE            %prec UNBOUNDED    { $$ = false; }
+            | /* EMPTY */                 %prec UNBOUNDED    { $$ = false; }
         ;

 json_name_and_value_list: