Fix SQL:2008 FETCH FIRST syntax to allow parameters.

OFFSET <x> ROWS FETCH FIRST <y> ROWS ONLY syntax is supposed to accept
<simple value specification>, which includes parameters as well as
literals. When this syntax was added all those years ago, it was done
inconsistently, with <x> and <y> being different subsets of the
standard syntax.

Rectify that by making <x> and <y> accept the same thing, and allowing
either a (signed) numeric literal or a c_expr there, which allows for
parameters, variables, and parenthesized arbitrary expressions.

Per bug #15200 from Lukas Eder.

Backpatch all the way, since this has been broken from the start.

Discussion: https://postgr.es/m/877enz476l.fsf@news-spur.riddles.org.uk
Discussion: http://postgr.es/m/152647780335.27204.16895288237122418685@wrigleys.postgresql.org

diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 57f11e66fb6dfa42706f8f216aba8c9e888aa955..4c8b6f1e73d31076f320502e272f500b7ae3d478 100644 (file)
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -1349,12 +1349,14 @@ OFFSET <replaceable class="parameter">start</replaceable>
 OFFSET <replaceable class="parameter">start</replaceable> { ROW | ROWS }
 FETCH { FIRST | NEXT } [ <replaceable class="parameter">count</replaceable> ] { ROW | ROWS } ONLY
 </synopsis>
-    In this syntax, to write anything except a simple integer constant for
-    <replaceable class="parameter">start</> or <replaceable
-    class="parameter">count</replaceable>, you must write parentheses
-    around it.
-    If <replaceable class="parameter">count</> is
-    omitted in a <literal>FETCH</> clause, it defaults to 1.
+    In this syntax, the <replaceable class="parameter">start</replaceable>
+    or <replaceable class="parameter">count</replaceable> value is required by
+    the standard to be a literal constant, a parameter, or a variable name;
+    as a <productname>PostgreSQL</productname> extension, other expressions
+    are allowed, but will generally need to be enclosed in parentheses to avoid
+    ambiguity.
+    If <replaceable class="parameter">count</replaceable> is
+    omitted in a <literal>FETCH</literal> clause, it defaults to 1.
     <literal>ROW</literal>
     and <literal>ROWS</literal> as well as <literal>FIRST</literal>
     and <literal>NEXT</literal> are noise words that don't influence

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 43e207a2337a28524dcf2b8181891d87248ca1c4..025e82b30fa4f1f2dabccad9eab6aaa40281e01c 100644 (file)
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -447,7 +447,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
 
 %type <node>   fetch_args limit_clause select_limit_value
                                offset_clause select_offset_value
-                               select_offset_value2 opt_select_fetch_first_value
+                               select_fetch_first_value I_or_F_const
 %type <ival>   row_or_rows first_or_next
 
 %type <list>   OptSeqOptList SeqOptList OptParenthesizedSeqOptList
@@ -11185,15 +11185,23 @@ limit_clause:
                                                         parser_errposition(@1)));
                                }
                        /* SQL:2008 syntax */
-                       | FETCH first_or_next opt_select_fetch_first_value row_or_rows ONLY
+                       /* to avoid shift/reduce conflicts, handle the optional value with
+                        * a separate production rather than an opt_ expression.  The fact
+                        * that ONLY is fully reserved means that this way, we defer any
+                        * decision about what rule reduces ROW or ROWS to the point where
+                        * we can see the ONLY token in the lookahead slot.
+                        */
+                       | FETCH first_or_next select_fetch_first_value row_or_rows ONLY
                                { $$ = $3; }
+                       | FETCH first_or_next row_or_rows ONLY
+                               { $$ = makeIntConst(1, -1); }
                ;
 
 offset_clause:
                        OFFSET select_offset_value
                                { $$ = $2; }
                        /* SQL:2008 syntax */
-                       | OFFSET select_offset_value2 row_or_rows
+                       | OFFSET select_fetch_first_value row_or_rows
                                { $$ = $2; }
                ;
 
@@ -11212,22 +11220,31 @@ select_offset_value:
 
 /*
  * Allowing full expressions without parentheses causes various parsing
- * problems with the trailing ROW/ROWS key words.  SQL only calls for
- * constants, so we allow the rest only with parentheses.  If omitted,
- * default to 1.
+ * problems with the trailing ROW/ROWS key words.  SQL spec only calls for
+ * <simple value specification>, which is either a literal or a parameter (but
+ * an <SQL parameter reference> could be an identifier, bringing up conflicts
+ * with ROW/ROWS). We solve this by leveraging the presence of ONLY (see above)
+ * to determine whether the expression is missing rather than trying to make it
+ * optional in this rule.
+ *
+ * c_expr covers almost all the spec-required cases (and more), but it doesn't
+ * cover signed numeric literals, which are allowed by the spec. So we include
+ * those here explicitly. We need FCONST as well as ICONST because values that
+ * don't fit in the platform's "long", but do fit in bigint, should still be
+ * accepted here. (This is possible in 64-bit Windows as well as all 32-bit
+ * builds.)
  */
-opt_select_fetch_first_value:
-                       SignedIconst                                            { $$ = makeIntConst($1, @1); }
-                       | '(' a_expr ')'                                        { $$ = $2; }
-                       | /*EMPTY*/                                                     { $$ = makeIntConst(1, -1); }
+select_fetch_first_value:
+                       c_expr                                                                  { $$ = $1; }
+                       | '+' I_or_F_const
+                               { $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "+", NULL, $2, @1); }
+                       | '-' I_or_F_const
+                               { $$ = doNegate($2, @1); }
                ;
 
-/*
- * Again, the trailing ROW/ROWS in this case prevent the full expression
- * syntax.  c_expr is the best we can do.
- */
-select_offset_value2:
-                       c_expr                                                                  { $$ = $1; }
+I_or_F_const:
+                       Iconst                                                                  { $$ = makeIntConst($1,@1); }
+                       | FCONST                                                                { $$ = makeFloatConst($1,@1); }
                ;
 
 /* noise words */