Fix parsing of underscores in pg_plan_advice occurrence numbers

The pg_plan_advice scanner recognizes underscores as digit separators
just like the core parser, but used strtoint() to convert occurrence
numbers which does not support underscores.  Consequently, advice such
as SEQ_SCAN(x#1_0) failed to parse.  Fix by using pg_strtoint32_safe()
like the core scanner, and also add regression test coverage.

This bug was independently found and reported by Lukas Fittl and Chao
Li.  Backpatch down to v19 where pg_plan_advice was introduced.

Author: Chao Li <lic@highgo.com>
Co-authored-by: Daniel Gustafsson <daniel@yesql.se>
Reported-by: Lukas Fittl <lukas@fittl.com>
Reported-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Reviewed-by: Lukas Fittl <lukas@fittl.com>
Discussion: https://postgr.es/m/22E2ECE0-B768-43D5-8575-61C3EBC2E4E8@gmail.com
Discussion: https://postgr.es/m/CAP53PkzKeD=t90OfeMsniYrcRe2THQbUx3g6wV17Y=ZtiwmWTQ@mail.gmail.com
Backpatch-through: 19

diff --git a/contrib/pg_plan_advice/expected/syntax.out b/contrib/pg_plan_advice/expected/syntax.out
index c61fd73a38559644f0d37f1f3bcc4706d4adf13d..3b57bb2bf57a48a65cf8afca10fb24d003a12ed2 100644 (file)
--- a/contrib/pg_plan_advice/expected/syntax.out
+++ b/contrib/pg_plan_advice/expected/syntax.out
@@ -65,6 +65,15 @@ EXPLAIN (COSTS OFF) SELECT 1;
    SEQ_SCAN(x#2) /* not matched */
 (3 rows)
 
+SET pg_plan_advice.advice = 'SEQ_SCAN(x#1_0)';
+EXPLAIN (COSTS OFF) SELECT 1;
+             QUERY PLAN             
+------------------------------------
+ Result
+ Supplied Plan Advice:
+   SEQ_SCAN(x#10) /* not matched */
+(3 rows)
+
 SET pg_plan_advice.advice = 'SEQ_SCAN (x/y)';
 EXPLAIN (COSTS OFF) SELECT 1;
             QUERY PLAN             
@@ -114,12 +123,19 @@ DETAIL:  Could not parse advice: syntax error at end of input
 SET pg_plan_advice.advice = 'SEQ_SCAN(#';
 ERROR:  invalid value for parameter "pg_plan_advice.advice": "SEQ_SCAN(#"
 DETAIL:  Could not parse advice: syntax error at or near "#"
+SET pg_plan_advice.advice = 'SEQ_SCAN(x#1_0_)';
+ERROR:  invalid value for parameter "pg_plan_advice.advice": "SEQ_SCAN(x#1_0_)"
+DETAIL:  Could not parse advice: trailing junk after numeric literal at or near "1_0_"
 SET pg_plan_advice.advice = '()';
 ERROR:  invalid value for parameter "pg_plan_advice.advice": "()"
 DETAIL:  Could not parse advice: syntax error at or near "("
 SET pg_plan_advice.advice = '123';
 ERROR:  invalid value for parameter "pg_plan_advice.advice": "123"
 DETAIL:  Could not parse advice: syntax error at or near "123"
+-- Out of range values.
+SET pg_plan_advice.advice = 'SEQ_SCAN(x#99999999999_99)';
+ERROR:  invalid value for parameter "pg_plan_advice.advice": "SEQ_SCAN(x#99999999999_99)"
+DETAIL:  Could not parse advice: integer out of range at or near "99999999999_99"
 -- Tags like SEQ_SCAN and NO_GATHER don't allow sublists at all; other tags,
 -- except for JOIN_ORDER, allow at most one level of sublist. Hence, these
 -- examples should error out.

diff --git a/contrib/pg_plan_advice/pgpa_scanner.l b/contrib/pg_plan_advice/pgpa_scanner.l
index e6d60f57e1e3ade589484bddaa050524203cf1a2..15d67cd48964d9962bde1bd3cbad08d3fca23228 100644 (file)
--- a/contrib/pg_plan_advice/pgpa_scanner.l
+++ b/contrib/pg_plan_advice/pgpa_scanner.l
@@ -8,9 +8,9 @@
  */
 #include "postgres.h"
 
-#include "common/string.h"
 #include "nodes/miscnodes.h"
 #include "parser/scansup.h"
+#include "utils/builtins.h"
 
 #include "pgpa_ast.h"
 #include "pgpa_parser.h"
@@ -82,6 +82,7 @@ identifier            {ident_start}{ident_cont}*
 
 decdigit               [0-9]
 decinteger             {decdigit}(_?{decdigit})*
+integer_junk   {decinteger}{identifier}
 
 space                  [ \t\n\r\f\v]
 whitespace             {space}+
@@ -136,16 +137,22 @@ xcinside          [^*/]+
                                }
 
 {decinteger}   {
-                                       char   *endptr;
+                                       ErrorSaveContext escontext = {T_ErrorSaveContext};
 
-                                       errno = 0;
-                                       yylval->integer = strtoint(yytext, &endptr, 10);
-                                       if (*endptr != '\0' || errno == ERANGE)
+                                       yylval->integer = pg_strtoint32_safe(yytext,
+                                                                                                                (Node *) &escontext);
+                                       if (escontext.error_occurred)
                                                pgpa_yyerror(result, parse_error_msg_p, yyscanner,
                                                                         "integer out of range");
                                        return TOK_INTEGER;
                                }
 
+{integer_junk} {
+                                       BEGIN(INITIAL);
+                                       pgpa_yyerror(result, parse_error_msg_p, yyscanner,
+                                                                "trailing junk after numeric literal");
+                               }
+
 {xcstart}              {
                                        BEGIN(xc);
                                }

diff --git a/contrib/pg_plan_advice/sql/syntax.sql b/contrib/pg_plan_advice/sql/syntax.sql
index 3f94b5f8bf3c6def6fb571e96368ab3b14b2fb05..5af7607db42e511fcedf85115c6e8d9dc8822383 100644 (file)
--- a/contrib/pg_plan_advice/sql/syntax.sql
+++ b/contrib/pg_plan_advice/sql/syntax.sql
@@ -19,6 +19,8 @@ SET pg_plan_advice.advice = 'seq_scan(x@y)';
 EXPLAIN (COSTS OFF) SELECT 1;
 SET pg_plan_advice.advice = 'SEQ_scan(x#2)';
 EXPLAIN (COSTS OFF) SELECT 1;
+SET pg_plan_advice.advice = 'SEQ_SCAN(x#1_0)';
+EXPLAIN (COSTS OFF) SELECT 1;
 SET pg_plan_advice.advice = 'SEQ_SCAN (x/y)';
 EXPLAIN (COSTS OFF) SELECT 1;
 SET pg_plan_advice.advice = '  SEQ_SCAN ( x / y . z )  ';
@@ -34,9 +36,13 @@ SET pg_plan_advice.advice = 'SEQ_SCAN("';
 SET pg_plan_advice.advice = 'SEQ_SCAN("")';
 SET pg_plan_advice.advice = 'SEQ_SCAN("a"';
 SET pg_plan_advice.advice = 'SEQ_SCAN(#';
+SET pg_plan_advice.advice = 'SEQ_SCAN(x#1_0_)';
 SET pg_plan_advice.advice = '()';
 SET pg_plan_advice.advice = '123';
 
+-- Out of range values.
+SET pg_plan_advice.advice = 'SEQ_SCAN(x#99999999999_99)';
+
 -- Tags like SEQ_SCAN and NO_GATHER don't allow sublists at all; other tags,
 -- except for JOIN_ORDER, allow at most one level of sublist. Hence, these
 -- examples should error out.