Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit5e9d8be

Browse files
committed
Fix plpgsql's handling of -- comments following expressions.
Up to now, read_sql_construct() has collected all the source text fromthe statement or expression's initial token up to the character justbefore the "until" token. It normally tries to strip trailingwhitespace from that, largely for neatness. If there was a "-- text"comment after the expression, this resulted in removing the newlinethat terminates the comment, which creates a hazard if we try to pastethe collected text into a larger SQL construct without inserting anewline after it. In particular this caused our handling of CASEconstructs to fail if there's a comment after a WHEN expression.Commit4adead1 noticed a similar problem with cursor arguments,and worked around it through the rather crude hack of suppressingthe whitespace-trimming behavior for those. Rather than do thatand leave the hazard open for future hackers to trip over, let'sfix it properly. pl_scanner.c already has enough infrastructureto report the end location of the expression's last token, sowe can copy up to that location and never collect any trailingwhitespace or comment to begin with.Erik Wienhold and Tom Lane, per report from Michal Bartak.Back-patch to all supported branches.Discussion:https://postgr.es/m/CAAVzF_FjRoi8fOVuLCZhQJx6HATQ7MKm=aFOHWZODFnLmjX-xA@mail.gmail.com
1 parentbfed705 commit5e9d8be

File tree

7 files changed

+74
-34
lines changed

7 files changed

+74
-34
lines changed

‎src/pl/plpgsql/src/expected/plpgsql_control.out

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,3 +681,20 @@ select case_test(13);
681681
other
682682
(1 row)
683683

684+
-- test line comment between WHEN and THEN
685+
create or replace function case_comment(int) returns text as $$
686+
begin
687+
case $1
688+
when 1 -- comment before THEN
689+
then return 'one';
690+
else
691+
return 'other';
692+
end case;
693+
end;
694+
$$ language plpgsql immutable;
695+
select case_comment(1);
696+
case_comment
697+
--------------
698+
one
699+
(1 row)
700+

‎src/pl/plpgsql/src/pl_gram.y

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ staticPLpgSQL_expr*read_sql_construct(int until,
7070
constchar *sqlstart,
7171
bool isexpression,
7272
bool valid_sql,
73-
bool trim,
7473
int *startloc,
7574
int *endtoken);
7675
staticPLpgSQL_expr*read_sql_expression(int until,
@@ -1463,7 +1462,6 @@ for_control: for_variable K_IN
14631462
"SELECT",
14641463
true,
14651464
false,
1466-
true,
14671465
&expr1loc,
14681466
&tok);
14691467

@@ -1870,7 +1868,7 @@ stmt_raise: K_RAISE
18701868
expr =read_sql_construct(',',';', K_USING,
18711869
", or ; or USING",
18721870
"SELECT",
1873-
true,true,true,
1871+
true,true,
18741872
NULL, &tok);
18751873
new->params =lappend(new->params, expr);
18761874
}
@@ -2001,7 +1999,7 @@ stmt_dynexecute : K_EXECUTE
20011999
expr =read_sql_construct(K_INTO, K_USING,';',
20022000
"INTO or USING or ;",
20032001
"SELECT",
2004-
true,true,true,
2002+
true,true,
20052003
NULL, &endtoken);
20062004

20072005
new =palloc(sizeof(PLpgSQL_stmt_dynexecute));
@@ -2040,7 +2038,7 @@ stmt_dynexecute : K_EXECUTE
20402038
expr =read_sql_construct(',',';', K_INTO,
20412039
", or ; or INTO",
20422040
"SELECT",
2043-
true,true,true,
2041+
true,true,
20442042
NULL, &endtoken);
20452043
new->params =lappend(new->params, expr);
20462044
}while (endtoken ==',');
@@ -2655,7 +2653,7 @@ static PLpgSQL_expr *
26552653
read_sql_expression(int until, const char *expected)
26562654
{
26572655
return read_sql_construct(until, 0, 0, expected,
2658-
"SELECT ", true, true,true,NULL, NULL);
2656+
"SELECT ", true, true, NULL, NULL);
26592657
}
26602658

26612659
/* Convenience routine to read an expression with two possible terminators*/
@@ -2664,15 +2662,15 @@ read_sql_expression2(int until, int until2, const char *expected,
26642662
int *endtoken)
26652663
{
26662664
return read_sql_construct(until, until2, 0, expected,
2667-
"SELECT ", true, true,true,NULL, endtoken);
2665+
"SELECT ", true, true, NULL, endtoken);
26682666
}
26692667

26702668
/* Convenience routine to read a SQL statement that must end with ';'*/
26712669
static PLpgSQL_expr *
26722670
read_sql_stmt(const char *sqlstart)
26732671
{
26742672
return read_sql_construct(';', 0, 0, ";",
2675-
sqlstart, false, true,true,NULL, NULL);
2673+
sqlstart, false, true, NULL, NULL);
26762674
}
26772675

26782676
/*
@@ -2685,7 +2683,6 @@ read_sql_stmt(const char *sqlstart)
26852683
* sqlstart:text to prefix to the accumulated SQL text
26862684
* isexpression: whether to say we're reading an "expression" or a "statement"
26872685
* valid_sql: whether to check the syntax of the expr (prefixed with sqlstart)
2688-
* trim:trim trailing whitespace
26892686
* startloc:if not NULL, location of first token is stored at *startloc
26902687
* endtoken:if not NULL, ending token is stored at *endtoken
26912688
*(this is only interesting if until2 or until3 isn't zero)
@@ -2698,14 +2695,14 @@ read_sql_construct(int until,
26982695
const char *sqlstart,
26992696
bool isexpression,
27002697
bool valid_sql,
2701-
bool trim,
27022698
int *startloc,
27032699
int *endtoken)
27042700
{
27052701
inttok;
27062702
StringInfoDatads;
27072703
IdentifierLookupsave_IdentifierLookup;
27082704
intstartlocation = -1;
2705+
intendlocation = -1;
27092706
intparenlevel = 0;
27102707
PLpgSQL_expr*expr;
27112708

@@ -2757,6 +2754,8 @@ read_sql_construct(int until,
27572754
expected),
27582755
parser_errposition(yylloc)));
27592756
}
2757+
/* Remember end+1 location of last accepted token*/
2758+
endlocation = yylloc + plpgsql_token_length();
27602759
}
27612760

27622761
plpgsql_IdentifierLookup = save_IdentifierLookup;
@@ -2767,22 +2766,22 @@ read_sql_construct(int until,
27672766
*endtoken = tok;
27682767

27692768
/* give helpful complaint about empty input*/
2770-
if (startlocation >=yylloc)
2769+
if (startlocation >=endlocation)
27712770
{
27722771
if (isexpression)
27732772
yyerror("missing expression");
27742773
else
27752774
yyerror("missing SQL statement");
27762775
}
27772776

2778-
plpgsql_append_source_text(&ds, startlocation, yylloc);
2779-
2780-
/* trim any trailing whitespace, for neatness*/
2781-
if (trim)
2782-
{
2783-
while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
2784-
ds.data[--ds.len] = '\0';
2785-
}
2777+
/*
2778+
* We save only the text from startlocation to endlocation-1. This
2779+
* suppresses the "until" token as well as any whitespace or comments
2780+
* following the last accepted token. (We used to strip such trailing
2781+
* whitespace by hand, but that causes problems if there's a "-- comment"
2782+
* in front of said whitespace.)
2783+
*/
2784+
plpgsql_append_source_text(&ds, startlocation, endlocation);
27862785

27872786
expr = palloc0(sizeof(PLpgSQL_expr));
27882787
expr->query= pstrdup(ds.data);
@@ -3873,16 +3872,12 @@ read_cursor_args(PLpgSQL_var *cursor, int until, const char *expected)
38733872
* Read the value expression. To provide the user with meaningful
38743873
* parse error positions, we check the syntax immediately, instead of
38753874
* checking the final expression that may have the arguments
3876-
* reordered. Trailing whitespace must not be trimmed, because
3877-
* otherwise input of the form (param -- comment\n, param) would be
3878-
* translated into a form where the second parameter is commented
3879-
* out.
3875+
* reordered.
38803876
*/
38813877
item = read_sql_construct(',', ')', 0,
38823878
",\" or \")",
38833879
sqlstart,
38843880
true, true,
3885-
false,/* do not trim*/
38863881
NULL, &endtoken);
38873882

38883883
argv[argpos] = item->query + strlen(sqlstart);

‎src/pl/plpgsql/src/pl_scanner.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,8 @@ plpgsql_yylex(void)
184184
tok1=T_DATUM;
185185
else
186186
tok1=T_CWORD;
187+
/* Adjust token length to include A.B.C */
188+
aux1.leng=aux5.lloc-aux1.lloc+aux5.leng;
187189
}
188190
else
189191
{
@@ -197,6 +199,8 @@ plpgsql_yylex(void)
197199
tok1=T_DATUM;
198200
else
199201
tok1=T_CWORD;
202+
/* Adjust token length to include A.B */
203+
aux1.leng=aux3.lloc-aux1.lloc+aux3.leng;
200204
}
201205
}
202206
else
@@ -210,6 +214,8 @@ plpgsql_yylex(void)
210214
tok1=T_DATUM;
211215
else
212216
tok1=T_CWORD;
217+
/* Adjust token length to include A.B */
218+
aux1.leng=aux3.lloc-aux1.lloc+aux3.leng;
213219
}
214220
}
215221
else
@@ -298,6 +304,17 @@ plpgsql_yylex(void)
298304
returntok1;
299305
}
300306

307+
/*
308+
* Return the length of the token last returned by plpgsql_yylex().
309+
*
310+
* In the case of compound tokens, the length includes all the parts.
311+
*/
312+
int
313+
plpgsql_token_length(void)
314+
{
315+
returnplpgsql_yyleng;
316+
}
317+
301318
/*
302319
* Internal yylex function. This wraps the core lexer and adds one feature:
303320
* a token pushback stack. We also make a couple of trivial single-token

‎src/pl/plpgsql/src/plpgsql.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,6 +1311,7 @@ extern void plpgsql_dumptree(PLpgSQL_function *func);
13111311
*/
13121312
externintplpgsql_base_yylex(void);
13131313
externintplpgsql_yylex(void);
1314+
externintplpgsql_token_length(void);
13141315
externvoidplpgsql_push_back_token(inttoken);
13151316
externboolplpgsql_token_is_unreserved_keyword(inttoken);
13161317
externvoidplpgsql_append_source_text(StringInfobuf,

‎src/pl/plpgsql/src/sql/plpgsql_control.sql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,3 +486,17 @@ select case_test(1);
486486
select case_test(2);
487487
select case_test(12);
488488
select case_test(13);
489+
490+
-- test line comment between WHEN and THEN
491+
create or replacefunctioncase_comment(int) returnstextas $$
492+
begin
493+
case $1
494+
when1-- comment before THEN
495+
then return'one';
496+
else
497+
return'other';
498+
end case;
499+
end;
500+
$$ language plpgsql immutable;
501+
502+
select case_comment(1);

‎src/test/regress/expected/plpgsql.out

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2185,11 +2185,9 @@ select namedparmcursor_test7();
21852185
ERROR: division by zero
21862186
CONTEXT: SQL statement "SELECT 42/0 AS p1, 77 AS p2;"
21872187
PL/pgSQL function namedparmcursor_test7() line 6 at OPEN
2188-
-- check that line comments work correctly within the argument list (there
2189-
-- is some special handling of this case in the code: the newline after the
2190-
-- comment must be preserved when the argument-evaluating query is
2191-
-- constructed, otherwise the comment effectively comments out the next
2192-
-- argument, too)
2188+
-- check that line comments work correctly within the argument list
2189+
-- (this used to require a special hack in the code; it no longer does,
2190+
-- but let's keep the test anyway)
21932191
create function namedparmcursor_test8() returns int4 as $$
21942192
declare
21952193
c1 cursor (p1 int, p2 int) for

‎src/test/regress/sql/plpgsql.sql

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1895,11 +1895,9 @@ begin
18951895
end $$ language plpgsql;
18961896
select namedparmcursor_test7();
18971897

1898-
-- check that line comments work correctly within the argument list (there
1899-
-- is some special handling of this case in the code: the newline after the
1900-
-- comment must be preserved when the argument-evaluating query is
1901-
-- constructed, otherwise the comment effectively comments out the next
1902-
-- argument, too)
1898+
-- check that line comments work correctly within the argument list
1899+
-- (this used to require a special hack in the code; it no longer does,
1900+
-- but let's keep the test anyway)
19031901
createfunctionnamedparmcursor_test8() returns int4as $$
19041902
declare
19051903
c1 cursor (p1int, p2int) for

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp