From 7a0aa8d12ace1e5e73ccf9cbee88fdde255fe172 Mon Sep 17 00:00:00 2001 From: Andrew Gierth Date: Mon, 21 May 2018 17:02:17 +0100 Subject: [PATCH] Fix SQL:2008 FETCH FIRST syntax to allow parameters. OFFSET ROWS FETCH FIRST ROWS ONLY syntax is supposed to accept , which includes parameters as well as literals. When this syntax was added all those years ago, it was done inconsistently, with and being different subsets of the standard syntax. Rectify that by making and 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 --- doc/src/sgml/ref/select.sgml | 14 ++++++----- src/backend/parser/gram.y | 49 ++++++++++++++++++++++++------------ 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index 57f11e66fb..4c8b6f1e73 100644 --- a/doc/src/sgml/ref/select.sgml +++ b/doc/src/sgml/ref/select.sgml @@ -1349,12 +1349,14 @@ OFFSET start OFFSET start { ROW | ROWS } FETCH { FIRST | NEXT } [ count ] { ROW | ROWS } ONLY - In this syntax, to write anything except a simple integer constant for - start or count, you must write parentheses - around it. - If count is - omitted in a FETCH clause, it defaults to 1. + In this syntax, the start + or count value is required by + the standard to be a literal constant, a parameter, or a variable name; + as a PostgreSQL extension, other expressions + are allowed, but will generally need to be enclosed in parentheses to avoid + ambiguity. + If count is + omitted in a FETCH clause, it defaults to 1. ROW and ROWS as well as FIRST and NEXT are noise words that don't influence diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 9e170f7536..a0ef488003 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -419,7 +419,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type 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 row_or_rows first_or_next %type OptSeqOptList SeqOptList @@ -10414,15 +10414,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; } ; @@ -10441,22 +10449,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 + * , which is either a literal or a parameter (but + * an 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 */