Received: from malur.postgresql.org ([2a02:16a8:dc51::56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fKAPp-0005xJ-0N for pgsql-hackers@arkaria.postgresql.org; Sat, 19 May 2018 22:33:29 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1fKAPn-0000qM-TA for pgsql-hackers@arkaria.postgresql.org; Sat, 19 May 2018 22:33:27 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fKAPn-0000qC-Lp for pgsql-hackers@lists.postgresql.org; Sat, 19 May 2018 22:33:27 +0000 Received: from lungold.riddles.org.uk ([82.68.208.19]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1fKAPk-0003MZ-FU for pgsql-hackers@postgresql.org; Sat, 19 May 2018 22:33:27 +0000 Received: from [192.168.127.1] (port=25220 helo=caithnard.riddles.org.uk) by lungold.riddles.org.uk with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.88 (FreeBSD)) (envelope-from ) id 1fKAPi-000PxQ-Oi for pgsql-hackers@postgresql.org; Sat, 19 May 2018 22:33:22 +0000 Received: from [127.0.0.1] (port=26979 helo=caithnard.riddles.org.uk) by caithnard.riddles.org.uk with esmtp (Exim 4.89 (FreeBSD)) (envelope-from ) id 1fKAPi-0001ra-3E for pgsql-hackers@postgresql.org; Sat, 19 May 2018 22:33:22 +0000 From: Andrew Gierth To: pgsql-hackers@postgresql.org Subject: Fix for FETCH FIRST syntax problems Message-ID: <877enz476l.fsf@news-spur.riddles.org.uk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (berkeley-unix) Date: Sat, 19 May 2018 23:33:22 +0100 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --=-=-= Content-Type: text/plain Per bug #15200, our support for sql2008 FETCH FIRST syntax is incomplete to the extent that it should be regarded as a bug; the spec quite clearly allows parameters/host variables in addition to constants, but we don't. Attached is a draft patch for fixing that, which additionally fixes the ugly wart that OFFSET ROW/ROWS and FETCH FIRST [] ROW/ROWS ONLY had very different productions for ; both now accept c_expr there. Shift/reduce conflict is avoided by taking advantage of the fact that ONLY is a fully reserved word. I've checked that this change would not make it any harder to add (post-2008 features) WITH TIES or PERCENT in the event that someone wants to do that. I think a case can be made that this should be backpatched; thoughts? (While I can't find any visible change for existing working queries, one change that does occur is that FETCH FIRST -1 ROWS ONLY now returns a different error message; but that was already inconsistent with the error from OFFSET -1 ROWS.) -- Andrew (irc:RhodiumToad) --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=ff.patch Content-Description: fix FETCH FIRST syntax diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index b5d3d3a071..330adb8c37 100644 --- a/doc/src/sgml/ref/select.sgml +++ b/doc/src/sgml/ref/select.sgml @@ -1399,10 +1399,11 @@ 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 + In this syntax, to write anything except a simple integer constant, + parameter, or variable name for start or count, you must write parentheses - around it. + class="parameter">count, you should write parentheses + around it (this is a PostgreSQL extension). If count is omitted in a FETCH clause, it defaults to 1. ROW diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index babb62dae1..e441c555a4 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -451,7 +451,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 %type row_or_rows first_or_next %type OptSeqOptList SeqOptList OptParenthesizedSeqOptList @@ -11570,15 +11570,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; } ; @@ -11597,21 +11605,16 @@ 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. - */ -opt_select_fetch_first_value: - SignedIconst { $$ = makeIntConst($1, @1); } - | '(' a_expr ')' { $$ = $2; } - | /*EMPTY*/ { $$ = makeIntConst(1, -1); } - ; - -/* - * Again, the trailing ROW/ROWS in this case prevent the full expression - * syntax. c_expr is the best we can do. + * 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 presense of ONLY (see above) + * to determine whether the expression is missing rather than trying to make it + * optional in this rule. + * + * c_expr covers all the spec-required cases (and more). */ -select_offset_value2: +select_fetch_first_value: c_expr { $$ = $1; } ; --=-=-=--