diff --git a/src/pl/plpgsql/src/expected/plpgsql_control.out b/src/pl/plpgsql/src/expected/plpgsql_control.out index 328bd48586..ccd4f54704 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_control.out +++ b/src/pl/plpgsql/src/expected/plpgsql_control.out @@ -681,3 +681,20 @@ select case_test(13); other (1 row) +-- test line comment between WHEN and THEN +create or replace function case_comment(int) returns text as $$ +begin + case $1 + when 1 -- comment before THEN + then return 'one'; + else + return 'other'; + end case; +end; +$$ language plpgsql immutable; +select case_comment(1); + case_comment +-------------- + one +(1 row) + diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y index fd7d263d59..f70d1dd6cf 100644 --- a/src/pl/plpgsql/src/pl_gram.y +++ b/src/pl/plpgsql/src/pl_gram.y @@ -69,7 +69,6 @@ static PLpgSQL_expr *read_sql_construct(int until, RawParseMode parsemode, bool isexpression, bool valid_sql, - bool trim, int *startloc, int *endtoken); static PLpgSQL_expr *read_sql_expression(int until, @@ -921,7 +920,7 @@ stmt_perform : K_PERFORM */ new->expr = read_sql_construct(';', 0, 0, ";", RAW_PARSE_DEFAULT, - false, false, true, + false, false, &startloc, NULL); /* overwrite "perform" ... */ memcpy(new->expr->query, " SELECT", 7); @@ -1007,7 +1006,7 @@ stmt_assign : T_DATUM plpgsql_push_back_token(T_DATUM); new->expr = read_sql_construct(';', 0, 0, ";", pmode, - false, true, true, + false, true, NULL, NULL); $$ = (PLpgSQL_stmt *)new; @@ -1496,7 +1495,6 @@ for_control : for_variable K_IN RAW_PARSE_DEFAULT, true, false, - true, &expr1loc, &tok); @@ -1901,7 +1899,7 @@ stmt_raise : K_RAISE expr = read_sql_construct(',', ';', K_USING, ", or ; or USING", RAW_PARSE_PLPGSQL_EXPR, - true, true, true, + true, true, NULL, &tok); new->params = lappend(new->params, expr); } @@ -2034,7 +2032,7 @@ stmt_dynexecute : K_EXECUTE expr = read_sql_construct(K_INTO, K_USING, ';', "INTO or USING or ;", RAW_PARSE_PLPGSQL_EXPR, - true, true, true, + true, true, NULL, &endtoken); new = palloc(sizeof(PLpgSQL_stmt_dynexecute)); @@ -2073,7 +2071,7 @@ stmt_dynexecute : K_EXECUTE expr = read_sql_construct(',', ';', K_INTO, ", or ; or INTO", RAW_PARSE_PLPGSQL_EXPR, - true, true, true, + true, true, NULL, &endtoken); new->params = lappend(new->params, expr); } while (endtoken == ','); @@ -2656,7 +2654,7 @@ read_sql_expression(int until, const char *expected) { return read_sql_construct(until, 0, 0, expected, RAW_PARSE_PLPGSQL_EXPR, - true, true, true, NULL, NULL); + true, true, NULL, NULL); } /* Convenience routine to read an expression with two possible terminators */ @@ -2666,7 +2664,7 @@ read_sql_expression2(int until, int until2, const char *expected, { return read_sql_construct(until, until2, 0, expected, RAW_PARSE_PLPGSQL_EXPR, - true, true, true, NULL, endtoken); + true, true, NULL, endtoken); } /* Convenience routine to read a SQL statement that must end with ';' */ @@ -2675,7 +2673,7 @@ read_sql_stmt(void) { return read_sql_construct(';', 0, 0, ";", RAW_PARSE_DEFAULT, - false, true, true, NULL, NULL); + false, true, NULL, NULL); } /* @@ -2688,7 +2686,6 @@ read_sql_stmt(void) * parsemode: raw_parser() mode to use * isexpression: whether to say we're reading an "expression" or a "statement" * valid_sql: whether to check the syntax of the expr - * trim: trim trailing whitespace * startloc: if not NULL, location of first token is stored at *startloc * endtoken: if not NULL, ending token is stored at *endtoken * (this is only interesting if until2 or until3 isn't zero) @@ -2701,7 +2698,6 @@ read_sql_construct(int until, RawParseMode parsemode, bool isexpression, bool valid_sql, - bool trim, int *startloc, int *endtoken) { @@ -2709,6 +2705,7 @@ read_sql_construct(int until, StringInfoData ds; IdentifierLookup save_IdentifierLookup; int startlocation = -1; + int endlocation = -1; int parenlevel = 0; PLpgSQL_expr *expr; @@ -2759,6 +2756,8 @@ read_sql_construct(int until, expected), parser_errposition(yylloc))); } + /* Remember end+1 location of last accepted token */ + endlocation = yylloc + plpgsql_token_length(); } plpgsql_IdentifierLookup = save_IdentifierLookup; @@ -2769,7 +2768,7 @@ read_sql_construct(int until, *endtoken = tok; /* give helpful complaint about empty input */ - if (startlocation >= yylloc) + if (startlocation >= endlocation) { if (isexpression) yyerror("missing expression"); @@ -2777,14 +2776,14 @@ read_sql_construct(int until, yyerror("missing SQL statement"); } - plpgsql_append_source_text(&ds, startlocation, yylloc); - - /* trim any trailing whitespace, for neatness */ - if (trim) - { - while (ds.len > 0 && scanner_isspace(ds.data[ds.len - 1])) - ds.data[--ds.len] = '\0'; - } + /* + * We save only the text from startlocation to endlocation-1. This + * suppresses the "until" token as well as any whitespace or comments + * following the last accepted token. (We used to strip such trailing + * whitespace by hand, but that causes problems if there's a "-- comment" + * in front of said whitespace.) + */ + plpgsql_append_source_text(&ds, startlocation, endlocation); expr = palloc0(sizeof(PLpgSQL_expr)); expr->query = pstrdup(ds.data); @@ -3923,16 +3922,12 @@ read_cursor_args(PLpgSQL_var *cursor, int until) * Read the value expression. To provide the user with meaningful * parse error positions, we check the syntax immediately, instead of * checking the final expression that may have the arguments - * reordered. Trailing whitespace must not be trimmed, because - * otherwise input of the form (param -- comment\n, param) would be - * translated into a form where the second parameter is commented - * out. + * reordered. */ item = read_sql_construct(',', ')', 0, ",\" or \")", RAW_PARSE_PLPGSQL_EXPR, true, true, - false, /* do not trim */ NULL, &endtoken); argv[argpos] = item->query; diff --git a/src/pl/plpgsql/src/pl_scanner.c b/src/pl/plpgsql/src/pl_scanner.c index e4c7a91ab5..5f51222f37 100644 --- a/src/pl/plpgsql/src/pl_scanner.c +++ b/src/pl/plpgsql/src/pl_scanner.c @@ -184,6 +184,8 @@ plpgsql_yylex(void) tok1 = T_DATUM; else tok1 = T_CWORD; + /* Adjust token length to include A.B.C */ + aux1.leng = aux5.lloc - aux1.lloc + aux5.leng; } else { @@ -197,6 +199,8 @@ plpgsql_yylex(void) tok1 = T_DATUM; else tok1 = T_CWORD; + /* Adjust token length to include A.B */ + aux1.leng = aux3.lloc - aux1.lloc + aux3.leng; } } else @@ -210,6 +214,8 @@ plpgsql_yylex(void) tok1 = T_DATUM; else tok1 = T_CWORD; + /* Adjust token length to include A.B */ + aux1.leng = aux3.lloc - aux1.lloc + aux3.leng; } } else @@ -298,6 +304,17 @@ plpgsql_yylex(void) return tok1; } +/* + * Return the length of the token last returned by plpgsql_yylex(). + * + * In the case of compound tokens, the length includes all the parts. + */ +int +plpgsql_token_length(void) +{ + return plpgsql_yyleng; +} + /* * Internal yylex function. This wraps the core lexer and adds one feature: * a token pushback stack. We also make a couple of trivial single-token diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 1d0f673e7b..74c07da60a 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -1302,6 +1302,7 @@ extern void plpgsql_dumptree(PLpgSQL_function *func); */ extern int plpgsql_base_yylex(void); extern int plpgsql_yylex(void); +extern int plpgsql_token_length(void); extern void plpgsql_push_back_token(int token); extern bool plpgsql_token_is_unreserved_keyword(int token); extern void plpgsql_append_source_text(StringInfo buf, diff --git a/src/pl/plpgsql/src/sql/plpgsql_control.sql b/src/pl/plpgsql/src/sql/plpgsql_control.sql index ed7231134f..8e007c51dc 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_control.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_control.sql @@ -486,3 +486,17 @@ select case_test(1); select case_test(2); select case_test(12); select case_test(13); + +-- test line comment between WHEN and THEN +create or replace function case_comment(int) returns text as $$ +begin + case $1 + when 1 -- comment before THEN + then return 'one'; + else + return 'other'; + end case; +end; +$$ language plpgsql immutable; + +select case_comment(1); diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 278d056505..b785c23b06 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -2363,11 +2363,9 @@ select namedparmcursor_test7(); ERROR: division by zero CONTEXT: SQL expression "42/0 AS p1, 77 AS p2" PL/pgSQL function namedparmcursor_test7() line 6 at OPEN --- check that line comments work correctly within the argument list (there --- is some special handling of this case in the code: the newline after the --- comment must be preserved when the argument-evaluating query is --- constructed, otherwise the comment effectively comments out the next --- argument, too) +-- check that line comments work correctly within the argument list +-- (this used to require a special hack in the code; it no longer does, +-- but let's keep the test anyway) create function namedparmcursor_test8() returns int4 as $$ declare c1 cursor (p1 int, p2 int) for diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index 7e52d4745d..741026eeb1 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -2023,11 +2023,9 @@ begin end $$ language plpgsql; select namedparmcursor_test7(); --- check that line comments work correctly within the argument list (there --- is some special handling of this case in the code: the newline after the --- comment must be preserved when the argument-evaluating query is --- constructed, otherwise the comment effectively comments out the next --- argument, too) +-- check that line comments work correctly within the argument list +-- (this used to require a special hack in the code; it no longer does, +-- but let's keep the test anyway) create function namedparmcursor_test8() returns int4 as $$ declare c1 cursor (p1 int, p2 int) for