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

Commit14e5680

Browse files
committed
Improve parser's reporting of statement start locations.
Up to now, the parser's reporting of a statement's stmt_locationincluded any preceding whitespace or comments. This isn't reallydesirable but was done to avoid accounting honestly for nonterminalsthat reduce to empty. It causes problems for pg_stat_statements,which partially compensates by manually stripping whitespace, butis not bright enough to strip /*-style comments. There will bemore problems with an upcoming patch to improve reporting of errorsin extension scripts, so it's time to do something about this.The thing we have to do to make it work right is to adjustYYLLOC_DEFAULT to scan the inputs of each production to find thefirst one that has a valid location (i.e., did not reduce toempty). In theory this adds a little bit of per-reduction overhead,but in practice it's negligible. I checked by measuring the timeto run raw_parser() on the contents of information_schema.sql, andthere was basically no change.Having done that, we can rely on any nonterminal that didn't reduceto completely empty to have a correct starting location, and we don'tneed the kluges the stmtmulti production formerly used.This should have a side benefit of allowing parse error reports toinclude an error position in some cases where they formerly failed todo so, due to trying to report the position of an empty nonterminal.I did not go looking for an example though. The one previously knowncase where that could happen (OptSchemaEltList) no longer needs thekluge it had; but I rather doubt that that was the only case.Discussion:https://postgr.es/m/ZvV1ClhnbJLCz7Sm@msg.df7cb.de
1 parent7c4d3fe commit14e5680

File tree

4 files changed

+34
-46
lines changed

4 files changed

+34
-46
lines changed

‎contrib/pg_stat_statements/expected/select.out

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@ SELECT 1 AS "int";
1919
1
2020
(1 row)
2121

22+
/* this comment should not appear in the output */
2223
SELECT 'hello'
23-
--multiline
24+
--but this one will appear
2425
AS "text";
2526
text
2627
-------
@@ -129,7 +130,7 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
129130
-------+------+------------------------------------------------------------------------------
130131
1 | 1 | PREPARE pgss_test (int) AS SELECT $1, $2 LIMIT $3
131132
4 | 4 | SELECT $1 +
132-
| | --multiline +
133+
| | --but this one will appear +
133134
| | AS "text"
134135
2 | 2 | SELECT $1 + $2
135136
3 | 3 | SELECT $1 + $2 + $3 AS "add"

‎contrib/pg_stat_statements/sql/select.sql

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ SELECT pg_stat_statements_reset() IS NOT NULL AS t;
1212
--
1313
SELECT1AS"int";
1414

15+
/* this comment should not appear in the output*/
1516
SELECT'hello'
16-
--multiline
17+
--but this one will appear
1718
AS"text";
1819

1920
SELECT'world'AS"text";

‎src/backend/nodes/queryjumblefuncs.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,12 @@ CleanQuerytext(const char *query, int *location, int *len)
9090
/*
9191
* Discard leading and trailing whitespace, too. Use scanner_isspace()
9292
* not libc's isspace(), because we want to match the lexer's behavior.
93+
*
94+
* Note: the parser now strips leading comments and whitespace from the
95+
* reported stmt_location, so this first loop will only iterate in the
96+
* unusual case that the location didn't propagate to here. But the
97+
* statement length will extend to the end-of-string or terminating
98+
* semicolon, so the second loop often does something useful.
9399
*/
94100
while (query_len>0&&scanner_isspace(query[0]))
95101
query++,query_location++,query_len--;

‎src/backend/parser/gram.y

Lines changed: 23 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -67,39 +67,25 @@
6767

6868

6969
/*
70-
* Location tracking support --- simpler than bison's default, since we only
71-
* want to track the start position not the end position of each nonterminal.
70+
* Location tracking support. Unlike bison's default, we only want
71+
* to track the start position not the end position of each nonterminal.
72+
* Nonterminals that reduce to empty receive position "-1". Since a
73+
* production's leading RHS nonterminal(s) may have reduced to empty,
74+
* we have to scan to find the first one that's not -1.
7275
*/
7376
#defineYYLLOC_DEFAULT(Current, Rhs, N) \
7477
do { \
75-
if ((N) >0) \
76-
(Current) = (Rhs)[1]; \
77-
else \
78-
(Current) = (-1); \
78+
(Current) = (-1); \
79+
for (int _i =1; _i <= (N); _i++) \
80+
{ \
81+
if ((Rhs)[_i] >=0) \
82+
{ \
83+
(Current) = (Rhs)[_i]; \
84+
break; \
85+
} \
86+
} \
7987
}while (0)
8088

81-
/*
82-
* The above macro assigns -1 (unknown) as the parse location of any
83-
* nonterminal that was reduced from an empty rule, or whose leftmost
84-
* component was reduced from an empty rule. This is problematic
85-
* for nonterminals defined like
86-
*OptFooList: / * EMPTY * / { ... } | OptFooList Foo { ... } ;
87-
* because we'll set -1 as the location during the first reduction and then
88-
* copy it during each subsequent reduction, leaving us with -1 for the
89-
* location even when the list is not empty. To fix that, do this in the
90-
* action for the nonempty rule(s):
91-
*if (@$ < 0) @$ = @2;
92-
* (Although we have many nonterminals that follow this pattern, we only
93-
* bother with fixing @$ like this when the nonterminal's parse location
94-
* is actually referenced in some rule.)
95-
*
96-
* A cleaner answer would be to make YYLLOC_DEFAULT scan all the Rhs
97-
* locations until it's found one that's not -1. Then we'd get a correct
98-
* location for any nonterminal that isn't entirely empty. But this way
99-
* would add overhead to every rule reduction, and so far there's not been
100-
* a compelling reason to pay that overhead.
101-
*/
102-
10389
/*
10490
* Bison doesn't allocate anything that needs to live across parser calls,
10591
* so we can easily have it use palloc instead of malloc. This prevents
@@ -930,43 +916,39 @@ parse_toplevel:
930916
|MODE_PLPGSQL_EXPRPLpgSQL_Expr
931917
{
932918
pg_yyget_extra(yyscanner)->parsetree =
933-
list_make1(makeRawStmt($2,0));
919+
list_make1(makeRawStmt($2,@2));
934920
}
935921
|MODE_PLPGSQL_ASSIGN1PLAssignStmt
936922
{
937923
PLAssignStmt *n = (PLAssignStmt *)$2;
938924

939925
n->nnames =1;
940926
pg_yyget_extra(yyscanner)->parsetree =
941-
list_make1(makeRawStmt((Node *) n,0));
927+
list_make1(makeRawStmt((Node *) n,@2));
942928
}
943929
|MODE_PLPGSQL_ASSIGN2PLAssignStmt
944930
{
945931
PLAssignStmt *n = (PLAssignStmt *)$2;
946932

947933
n->nnames =2;
948934
pg_yyget_extra(yyscanner)->parsetree =
949-
list_make1(makeRawStmt((Node *) n,0));
935+
list_make1(makeRawStmt((Node *) n,@2));
950936
}
951937
|MODE_PLPGSQL_ASSIGN3PLAssignStmt
952938
{
953939
PLAssignStmt *n = (PLAssignStmt *)$2;
954940

955941
n->nnames =3;
956942
pg_yyget_extra(yyscanner)->parsetree =
957-
list_make1(makeRawStmt((Node *) n,0));
943+
list_make1(makeRawStmt((Node *) n,@2));
958944
}
959945
;
960946

961947
/*
962948
* At top level, we wrap each stmt with a RawStmt node carrying start location
963-
* and length of the stmt's text. Notice that the start loc/len are driven
964-
* entirely from semicolon locations (@2). It would seem natural to use
965-
* @1 or @3 to get the true start location of a stmt, but that doesn't work
966-
* for statements that can start with empty nonterminals (opt_with_clause is
967-
* the main offender here); as noted in the comments for YYLLOC_DEFAULT,
968-
* we'd get -1 for the location in such cases.
969-
* We also take care to discard empty statements entirely.
949+
* and length of the stmt's text.
950+
* We also take care to discard empty statements entirely (which among other
951+
* things dodges the problem of assigning them a location).
970952
*/
971953
stmtmulti:stmtmulti';'toplevel_stmt
972954
{
@@ -976,14 +958,14 @@ stmtmulti:stmtmulti ';' toplevel_stmt
976958
updateRawStmtEnd(llast_node(RawStmt, $1), @2);
977959
}
978960
if ($3 !=NULL)
979-
$$ = lappend($1, makeRawStmt($3,@2 +1));
961+
$$ = lappend($1, makeRawStmt($3,@3));
980962
else
981963
$$ =$1;
982964
}
983965
|toplevel_stmt
984966
{
985967
if ($1 !=NULL)
986-
$$ = list_make1(makeRawStmt($1,0));
968+
$$ = list_make1(makeRawStmt($1,@1));
987969
else
988970
$$ = NIL;
989971
}
@@ -1584,8 +1566,6 @@ CreateSchemaStmt:
15841566
OptSchemaEltList:
15851567
OptSchemaEltListschema_stmt
15861568
{
1587-
if (@$ <0)/* see comments for YYLLOC_DEFAULT*/
1588-
@$ =@2;
15891569
$$ = lappend($1,$2);
15901570
}
15911571
|/* EMPTY*/

0 commit comments

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