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

Commitd85db0a

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 parentef124d0 commitd85db0a

File tree

7 files changed

+76
-36
lines changed

7 files changed

+76
-36
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: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ staticPLpgSQL_expr*read_sql_construct(int until,
6969
RawParseMode parsemode,
7070
bool isexpression,
7171
bool valid_sql,
72-
bool trim,
7372
int *startloc,
7473
int *endtoken);
7574
staticPLpgSQL_expr*read_sql_expression(int until,
@@ -923,7 +922,7 @@ stmt_perform: K_PERFORM
923922
*/
924923
new->expr =read_sql_construct(';',0,0,";",
925924
RAW_PARSE_DEFAULT,
926-
false,false,true,
925+
false,false,
927926
&startloc,NULL);
928927
/* overwrite "perform" ...*/
929928
memcpy(new->expr->query," SELECT",7);
@@ -1009,7 +1008,7 @@ stmt_assign: T_DATUM
10091008
plpgsql_push_back_token(T_DATUM);
10101009
new->expr =read_sql_construct(';',0,0,";",
10111010
pmode,
1012-
false,true,true,
1011+
false,true,
10131012
NULL,NULL);
10141013

10151014
$$ = (PLpgSQL_stmt *)new;
@@ -1498,7 +1497,6 @@ for_control: for_variable K_IN
14981497
RAW_PARSE_DEFAULT,
14991498
true,
15001499
false,
1501-
true,
15021500
&expr1loc,
15031501
&tok);
15041502

@@ -1903,7 +1901,7 @@ stmt_raise: K_RAISE
19031901
expr =read_sql_construct(',',';', K_USING,
19041902
", or ; or USING",
19051903
RAW_PARSE_PLPGSQL_EXPR,
1906-
true,true,true,
1904+
true,true,
19071905
NULL, &tok);
19081906
new->params =lappend(new->params, expr);
19091907
}
@@ -2040,7 +2038,7 @@ stmt_dynexecute : K_EXECUTE
20402038
expr =read_sql_construct(K_INTO, K_USING,';',
20412039
"INTO or USING or ;",
20422040
RAW_PARSE_PLPGSQL_EXPR,
2043-
true,true,true,
2041+
true,true,
20442042
NULL, &endtoken);
20452043

20462044
new =palloc(sizeof(PLpgSQL_stmt_dynexecute));
@@ -2079,7 +2077,7 @@ stmt_dynexecute : K_EXECUTE
20792077
expr =read_sql_construct(',',';', K_INTO,
20802078
", or ; or INTO",
20812079
RAW_PARSE_PLPGSQL_EXPR,
2082-
true,true,true,
2080+
true,true,
20832081
NULL, &endtoken);
20842082
new->params =lappend(new->params, expr);
20852083
}while (endtoken ==',');
@@ -2663,7 +2661,7 @@ read_sql_expression(int until, const char *expected)
26632661
{
26642662
return read_sql_construct(until, 0, 0, expected,
26652663
RAW_PARSE_PLPGSQL_EXPR,
2666-
true, true,true,NULL, NULL);
2664+
true, true, NULL, NULL);
26672665
}
26682666

26692667
/* Convenience routine to read an expression with two possible terminators*/
@@ -2673,7 +2671,7 @@ read_sql_expression2(int until, int until2, const char *expected,
26732671
{
26742672
return read_sql_construct(until, until2, 0, expected,
26752673
RAW_PARSE_PLPGSQL_EXPR,
2676-
true, true,true,NULL, endtoken);
2674+
true, true, NULL, endtoken);
26772675
}
26782676

26792677
/* Convenience routine to read a SQL statement that must end with ';'*/
@@ -2682,7 +2680,7 @@ read_sql_stmt(void)
26822680
{
26832681
return read_sql_construct(';', 0, 0, ";",
26842682
RAW_PARSE_DEFAULT,
2685-
false, true,true,NULL, NULL);
2683+
false, true, NULL, NULL);
26862684
}
26872685

26882686
/*
@@ -2695,7 +2693,6 @@ read_sql_stmt(void)
26952693
* parsemode:raw_parser() mode to use
26962694
* isexpression: whether to say we're reading an "expression" or a "statement"
26972695
* valid_sql: whether to check the syntax of the expr
2698-
* trim:trim trailing whitespace
26992696
* startloc:if not NULL, location of first token is stored at *startloc
27002697
* endtoken:if not NULL, ending token is stored at *endtoken
27012698
*(this is only interesting if until2 or until3 isn't zero)
@@ -2708,14 +2705,14 @@ read_sql_construct(int until,
27082705
RawParseMode parsemode,
27092706
bool isexpression,
27102707
bool valid_sql,
2711-
bool trim,
27122708
int *startloc,
27132709
int *endtoken)
27142710
{
27152711
inttok;
27162712
StringInfoData ds;
27172713
IdentifierLookup save_IdentifierLookup;
27182714
intstartlocation = -1;
2715+
intendlocation = -1;
27192716
intparenlevel = 0;
27202717
PLpgSQL_expr *expr;
27212718

@@ -2766,6 +2763,8 @@ read_sql_construct(int until,
27662763
expected),
27672764
parser_errposition(yylloc)));
27682765
}
2766+
/* Remember end+1 location of last accepted token*/
2767+
endlocation = yylloc + plpgsql_token_length();
27692768
}
27702769

27712770
plpgsql_IdentifierLookup = save_IdentifierLookup;
@@ -2776,22 +2775,22 @@ read_sql_construct(int until,
27762775
*endtoken = tok;
27772776

27782777
/* give helpful complaint about empty input*/
2779-
if (startlocation >=yylloc)
2778+
if (startlocation >=endlocation)
27802779
{
27812780
if (isexpression)
27822781
yyerror("missing expression");
27832782
else
27842783
yyerror("missing SQL statement");
27852784
}
27862785

2787-
plpgsql_append_source_text(&ds, startlocation, yylloc);
2788-
2789-
/* trim any trailing whitespace, for neatness*/
2790-
if (trim)
2791-
{
2792-
while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1]))
2793-
ds.data[--ds.len] = '\0';
2794-
}
2786+
/*
2787+
* We save only the text from startlocation to endlocation-1. This
2788+
* suppresses the "until" token as well as any whitespace or comments
2789+
* following the last accepted token. (We used to strip such trailing
2790+
* whitespace by hand, but that causes problems if there's a "-- comment"
2791+
* in front of said whitespace.)
2792+
*/
2793+
plpgsql_append_source_text(&ds, startlocation, endlocation);
27952794

27962795
expr = palloc0(sizeof(PLpgSQL_expr));
27972796
expr->query = pstrdup(ds.data);
@@ -3932,16 +3931,12 @@ read_cursor_args(PLpgSQL_var *cursor, int until)
39323931
* Read the value expression. To provide the user with meaningful
39333932
* parse error positions, we check the syntax immediately, instead of
39343933
* checking the final expression that may have the arguments
3935-
* reordered. Trailing whitespace must not be trimmed, because
3936-
* otherwise input of the form (param -- comment\n, param) would be
3937-
* translated into a form where the second parameter is commented
3938-
* out.
3934+
* reordered.
39393935
*/
39403936
item = read_sql_construct(',', ')', 0,
39413937
",\" or \")",
39423938
RAW_PARSE_PLPGSQL_EXPR,
39433939
true, true,
3944-
false,/* do not trim*/
39453940
NULL, &endtoken);
39463941

39473942
argv[argpos] = item->query;

‎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
@@ -1321,6 +1321,7 @@ extern void plpgsql_dumptree(PLpgSQL_function *func);
13211321
*/
13221322
externintplpgsql_base_yylex(void);
13231323
externintplpgsql_yylex(void);
1324+
externintplpgsql_token_length(void);
13241325
externvoidplpgsql_push_back_token(inttoken);
13251326
externboolplpgsql_token_is_unreserved_keyword(inttoken);
13261327
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
@@ -2390,11 +2390,9 @@ select namedparmcursor_test7();
23902390
ERROR: division by zero
23912391
CONTEXT: SQL expression "42/0 AS p1, 77 AS p2"
23922392
PL/pgSQL function namedparmcursor_test7() line 6 at OPEN
2393-
-- check that line comments work correctly within the argument list (there
2394-
-- is some special handling of this case in the code: the newline after the
2395-
-- comment must be preserved when the argument-evaluating query is
2396-
-- constructed, otherwise the comment effectively comments out the next
2397-
-- argument, too)
2393+
-- check that line comments work correctly within the argument list
2394+
-- (this used to require a special hack in the code; it no longer does,
2395+
-- but let's keep the test anyway)
23982396
create function namedparmcursor_test8() returns int4 as $$
23992397
declare
24002398
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
@@ -2047,11 +2047,9 @@ begin
20472047
end $$ language plpgsql;
20482048
select namedparmcursor_test7();
20492049

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

0 commit comments

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