I noticed that we (kind of) accept underscores in positional parameters. For example, this works:
=> PREPARE p1 AS SELECT $1_2; PREPARE => EXECUTE p1 (123); ?column? ---------- 123 (1 row) Parameter $1_2 is taken as $1 because in rule {param} in scan.l we get the parameter number with atol which stops at the underscore. That's a regression in faff8f8e47f. Before that commit, $1_2 resulted in "ERROR: trailing junk after parameter". I can't tell which fix is the way to go: (1) accept underscores without using atol, or (2) just forbid underscores. Any ideas? atol can be replaced with pg_strtoint32_safe to handle the underscores. This also avoids atol's undefined behavior on overflows. AFAICT, positional parameters are not part of the SQL standard, so nothing prevents us from accepting underscores here as well. The attached patch does that and also adds a test case. But reverting {param} to its old form to forbid underscores also makes sense. That is: param \${decdigit}+ param_junk \${decdigit}+{ident_start} It seems very unlikely that anybody uses that many parameters and still cares about readability to use underscores. But maybe users simply expect that underscores are valid here as well. -- Erik
>From 0f319818360cdd26e96198ea7fcaba1b8298f264 Mon Sep 17 00:00:00 2001 From: Erik Wienhold <e...@ewie.name> Date: Tue, 14 May 2024 04:14:08 +0200 Subject: [PATCH] Fix parsing of positional parameters with underscores Underscores were added to numeric literals in faff8f8e47. That also introduced a regression whereby positional parameters accepted underscores as well. But such parameter numbers were not parsed correctly by atol which stops at the first non-digit character. So parameter $1_2 would be taken as just $1. Fix that by replacing atol with pg_strtoint32_safe. Thereby we also avoid the undefined behavior of atol on overflows. --- src/backend/parser/scan.l | 5 ++++- src/test/regress/expected/numerology.out | 1 + src/test/regress/sql/numerology.sql | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/backend/parser/scan.l b/src/backend/parser/scan.l index 5eaadf53b2..ebb7ace9ca 100644 --- a/src/backend/parser/scan.l +++ b/src/backend/parser/scan.l @@ -992,7 +992,10 @@ other . {param} { SET_YYLLOC(); - yylval->ival = atol(yytext + 1); + ErrorSaveContext escontext = {T_ErrorSaveContext}; + yylval->ival = pg_strtoint32_safe(yytext + 1, (Node *) &escontext); + if (escontext.error_occurred) + yyerror("parameter too large"); return PARAM; } {param_junk} { diff --git a/src/test/regress/expected/numerology.out b/src/test/regress/expected/numerology.out index f662a5050a..3c1308e22c 100644 --- a/src/test/regress/expected/numerology.out +++ b/src/test/regress/expected/numerology.out @@ -297,6 +297,7 @@ SELECT 1_000.5e0_1; 10005 (1 row) +PREPARE p2 AS SELECT $0_1; -- error cases SELECT _100; ERROR: column "_100" does not exist diff --git a/src/test/regress/sql/numerology.sql b/src/test/regress/sql/numerology.sql index 1941c58e68..11af76828d 100644 --- a/src/test/regress/sql/numerology.sql +++ b/src/test/regress/sql/numerology.sql @@ -77,6 +77,8 @@ SELECT 1_000.; SELECT .000_005; SELECT 1_000.5e0_1; +PREPARE p2 AS SELECT $0_1; + -- error cases SELECT _100; SELECT 100_; -- 2.45.0