Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit cec25cc

Browse files
author
Commitfest Bot
committed
[CF 5733] v2 - Improve explicit cursor handling in pg_stat_statements
This branch was automatically generated by a robot using patches from an email thread registered at: https://commitfest.postgresql.org/patch/5733 The branch will be overwritten each time a new patch version is posted to the thread, and also periodically to check for bitrot caused by changes on the master branch. Patch(es): https://www.postgresql.org/message-id/CAA5RZ0v7VVdG-eoKcPjMnL6Dp=2ujPK2qk7Qx5NG_EPqd6naug@mail.gmail.com Author(s): Sami Imseih
2 parents e5a3c9d + cdfefb6 commit cec25cc

File tree

6 files changed

+97
-53
lines changed

6 files changed

+97
-53
lines changed

‎contrib/pg_stat_statements/expected/cursors.out

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,13 @@ CLOSE cursor_stats_1;
1616
DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT 2;
1717
CLOSE cursor_stats_1;
1818
SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
19-
calls | rows | query
20-
-------+------+-------------------------------------------------------
21-
2 | 0 | CLOSE cursor_stats_1
22-
2 | 0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
19+
calls | rows | query
20+
-------+------+----------------------------------------------------
21+
2 | 0 | CLOSE $1
22+
2 | 0 | DECLARE $1 CURSOR WITH HOLD FOR SELECT $2
23+
2 | 2 | SELECT $1
2324
1 | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
24-
(3 rows)
25+
(4 rows)
2526

2627
SELECT pg_stat_statements_reset() IS NOT NULL AS t;
2728
t
@@ -49,18 +50,17 @@ CLOSE cursor_stats_1;
4950
CLOSE cursor_stats_2;
5051
COMMIT;
5152
SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
52-
calls | rows | query
53-
-------+------+-------------------------------------------------------
53+
calls | rows | query
54+
-------+------+----------------------------------------------------
5455
1 | 0 | BEGIN
55-
1 | 0 | CLOSE cursor_stats_1
56-
1 | 0 | CLOSE cursor_stats_2
56+
2 | 0 | CLOSE $1
5757
1 | 0 | COMMIT
58-
1 | 0 | DECLARE cursor_stats_1 CURSOR WITH HOLD FOR SELECT $1
59-
1 | 0 | DECLARE cursor_stats_2 CURSOR WITH HOLD FOR SELECT $1
60-
1 | 1 | FETCH 1 IN cursor_stats_1
61-
1 | 1 | FETCH 1 IN cursor_stats_2
58+
2 | 0 | DECLARE $1 CURSOR WITH HOLD FOR SELECT $2
59+
1 | 1 | FETCH $1 IN cursor_stats_1
60+
1 | 1 | FETCH $1 IN cursor_stats_2
61+
2 | 2 | SELECT $1
6262
1 | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
63-
(9 rows)
63+
(8 rows)
6464

6565
SELECT pg_stat_statements_reset() IS NOT NULL AS t;
6666
t

‎contrib/pg_stat_statements/expected/level_tracking.out

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -921,7 +921,7 @@ SELECT toplevel, calls, query FROM pg_stat_statements
921921
toplevel | calls | query
922922
----------+-------+------------------------------------------------------------------------------
923923
t | 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF, BUFFERS OFF) +
924-
| | DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab
924+
| | DECLARE $1 CURSOR FOR SELECT * FROM stats_track_tab
925925
t | 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF, BUFFERS OFF) SELECT $1
926926
f | 1 | SELECT $1
927927
f | 1 | SELECT * FROM stats_track_tab
@@ -954,7 +954,7 @@ SELECT toplevel, calls, query FROM pg_stat_statements
954954
toplevel | calls | query
955955
----------+-------+------------------------------------------------------------------------------
956956
t | 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF, BUFFERS OFF) +
957-
| | DECLARE foocur CURSOR FOR SELECT * FROM stats_track_tab
957+
| | DECLARE $1 CURSOR FOR SELECT * FROM stats_track_tab
958958
t | 1 | EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF, BUFFERS OFF) SELECT $1
959959
t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
960960
(3 rows)
@@ -1136,14 +1136,14 @@ CLOSE foocur;
11361136
COMMIT;
11371137
SELECT toplevel, calls, query FROM pg_stat_statements
11381138
ORDER BY query COLLATE "C";
1139-
toplevel | calls | query
1140-
----------+-------+---------------------------------------------------------
1139+
toplevel | calls | query
1140+
----------+-------+-----------------------------------------------------
11411141
t | 1 | BEGIN
1142-
t | 1 | CLOSE foocur
1142+
t | 1 | CLOSE $1
11431143
t | 1 | COMMIT
1144-
t | 1 | DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab
1145-
t | 1 | FETCH FORWARD 1 FROM foocur
1146-
f | 1 | SELECT * from stats_track_tab
1144+
t | 1 | DECLARE $1 CURSOR FOR SELECT * from stats_track_tab
1145+
t | 1 | FETCH FORWARD $1 FROM foocur
1146+
t | 1 | SELECT * from stats_track_tab
11471147
t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
11481148
(7 rows)
11491149

@@ -1166,15 +1166,16 @@ CLOSE foocur;
11661166
COMMIT;
11671167
SELECT toplevel, calls, query FROM pg_stat_statements
11681168
ORDER BY query COLLATE "C";
1169-
toplevel | calls | query
1170-
----------+-------+---------------------------------------------------------
1169+
toplevel | calls | query
1170+
----------+-------+-----------------------------------------------------
11711171
t | 1 | BEGIN
1172-
t | 1 | CLOSE foocur
1172+
t | 1 | CLOSE $1
11731173
t | 1 | COMMIT
1174-
t | 1 | DECLARE FOOCUR CURSOR FOR SELECT * FROM stats_track_tab
1175-
t | 1 | FETCH FORWARD 1 FROM foocur
1174+
t | 1 | DECLARE $1 CURSOR FOR SELECT * FROM stats_track_tab
1175+
t | 1 | FETCH FORWARD $1 FROM foocur
1176+
t | 1 | SELECT * FROM stats_track_tab
11761177
t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
1177-
(6 rows)
1178+
(7 rows)
11781179

11791180
-- COPY - all-level tracking.
11801181
SET pg_stat_statements.track = 'all';

‎contrib/pg_stat_statements/expected/utility.out

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -701,14 +701,13 @@ SELECT calls, rows, query FROM pg_stat_statements ORDER BY query COLLATE "C";
701701
1 | 3 | COPY pgss_ctas (a, b) FROM STDIN
702702
1 | 13 | CREATE MATERIALIZED VIEW pgss_matv AS SELECT * FROM pgss_ctas
703703
1 | 10 | CREATE TABLE pgss_ctas AS SELECT a, $1 b FROM generate_series($2, $3) a
704-
1 | 0 | DECLARE pgss_cursor CURSOR FOR SELECT * FROM pgss_matv
705-
1 | 5 | FETCH FORWARD 5 pgss_cursor
706-
1 | 7 | FETCH FORWARD ALL pgss_cursor
707-
1 | 1 | FETCH NEXT pgss_cursor
704+
1 | 0 | DECLARE $1 CURSOR FOR SELECT * FROM pgss_matv
705+
3 | 13 | FETCH NEXT pgss_cursor
708706
1 | 13 | REFRESH MATERIALIZED VIEW pgss_matv
707+
1 | 13 | SELECT * FROM pgss_matv
709708
1 | 10 | SELECT generate_series($1, $2) c INTO pgss_select_into
710709
1 | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
711-
(12 rows)
710+
(11 rows)
712711

713712
DROP MATERIALIZED VIEW pgss_matv;
714713
DROP TABLE pgss_ctas;

‎contrib/pg_stat_statements/pg_stat_statements.c

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,11 @@ static bool pgss_save = true; /* whether to save stats across shutdown */
311311
SpinLockRelease(&pgss->mutex); \
312312
} while(0)
313313

314+
#define is_cursor_utility() \
315+
(IsA(parsetree, DeclareCursorStmt) || \
316+
IsA(parsetree, FetchStmt) || \
317+
IsA(parsetree, ClosePortalStmt))
318+
314319
/*---- Function declarations ----*/
315320

316321
PG_FUNCTION_INFO_V1(pg_stat_statements_reset);
@@ -1115,6 +1120,7 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
11151120
int saved_stmt_location = pstmt->stmt_location;
11161121
int saved_stmt_len = pstmt->stmt_len;
11171122
bool enabled = pgss_track_utility && pgss_enabled(nesting_level);
1123+
bool bump_level;
11181124

11191125
/*
11201126
* Force utility statements to get queryId zero. We do this even in cases
@@ -1145,8 +1151,14 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
11451151
* ensuing EXECUTEs. This would be confusing. Since PREPARE doesn't
11461152
* actually run the planner (only parse+rewrite), its costs are generally
11471153
* pretty negligible and it seems okay to just ignore it.
1154+
*
1155+
* If it's a cursor related utility command, don't bump the level and
1156+
* force the tracking of the underlying sql.
11481157
*/
1149-
if (enabled &&
1158+
bump_level =
1159+
!is_cursor_utility();
1160+
1161+
if ((enabled || !bump_level) &&
11501162
!IsA(parsetree, ExecuteStmt) &&
11511163
!IsA(parsetree, PrepareStmt))
11521164
{
@@ -1157,12 +1169,14 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
11571169
bufusage;
11581170
WalUsage walusage_start,
11591171
walusage;
1172+
bool store_utility = true;
11601173

11611174
bufusage_start = pgBufferUsage;
11621175
walusage_start = pgWalUsage;
11631176
INSTR_TIME_SET_CURRENT(start);
11641177

1165-
nesting_level++;
1178+
if (bump_level)
1179+
nesting_level++;
11661180
PG_TRY();
11671181
{
11681182
if (prev_ProcessUtility)
@@ -1176,7 +1190,8 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
11761190
}
11771191
PG_FINALLY();
11781192
{
1179-
nesting_level--;
1193+
if (bump_level)
1194+
nesting_level--;
11801195
}
11811196
PG_END_TRY();
11821197

@@ -1212,19 +1227,24 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
12121227
memset(&walusage, 0, sizeof(WalUsage));
12131228
WalUsageAccumDiff(&walusage, &pgWalUsage, &walusage_start);
12141229

1215-
pgss_store(queryString,
1216-
saved_queryId,
1217-
saved_stmt_location,
1218-
saved_stmt_len,
1219-
PGSS_EXEC,
1220-
INSTR_TIME_GET_MILLISEC(duration),
1221-
rows,
1222-
&bufusage,
1223-
&walusage,
1224-
NULL,
1225-
NULL,
1226-
0,
1227-
0);
1230+
/*
1231+
* In case we got here because of a cursor command, let's make sure we
1232+
* have utility tracking enabled, again.
1233+
*/
1234+
if (enabled)
1235+
pgss_store(queryString,
1236+
saved_queryId,
1237+
saved_stmt_location,
1238+
saved_stmt_len,
1239+
PGSS_EXEC,
1240+
INSTR_TIME_GET_MILLISEC(duration),
1241+
rows,
1242+
&bufusage,
1243+
&walusage,
1244+
NULL,
1245+
NULL,
1246+
0,
1247+
0);
12281248
}
12291249
else
12301250
{
@@ -1240,7 +1260,7 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
12401260
* To be absolutely certain we don't mess up the nesting level,
12411261
* evaluate the bump_level condition just once.
12421262
*/
1243-
bool bump_level =
1263+
bump_level =
12441264
!IsA(parsetree, ExecuteStmt) &&
12451265
!IsA(parsetree, PrepareStmt);
12461266

‎src/backend/parser/gram.y

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3341,13 +3341,15 @@ ClosePortalStmt:
33413341
ClosePortalStmt *n = makeNode(ClosePortalStmt);
33423342

33433343
n->portalname = $2;
3344+
n->location = @2;
33443345
$$ = (Node *) n;
33453346
}
33463347
| CLOSE ALL
33473348
{
33483349
ClosePortalStmt *n = makeNode(ClosePortalStmt);
33493350

33503351
n->portalname = NULL;
3352+
n->location = -1;
33513353
$$ = (Node *) n;
33523354
}
33533355
;
@@ -7479,6 +7481,7 @@ fetch_args: cursor_name
74797481
n->portalname = $1;
74807482
n->direction = FETCH_FORWARD;
74817483
n->howMany = 1;
7484+
n->location = -1;
74827485
$$ = (Node *) n;
74837486
}
74847487
| from_in cursor_name
@@ -7488,6 +7491,7 @@ fetch_args: cursor_name
74887491
n->portalname = $2;
74897492
n->direction = FETCH_FORWARD;
74907493
n->howMany = 1;
7494+
n->location = -1;
74917495
$$ = (Node *) n;
74927496
}
74937497
| NEXT opt_from_in cursor_name
@@ -7497,6 +7501,7 @@ fetch_args: cursor_name
74977501
n->portalname = $3;
74987502
n->direction = FETCH_FORWARD;
74997503
n->howMany = 1;
7504+
n->location = -1;
75007505
$$ = (Node *) n;
75017506
}
75027507
| PRIOR opt_from_in cursor_name
@@ -7506,6 +7511,7 @@ fetch_args: cursor_name
75067511
n->portalname = $3;
75077512
n->direction = FETCH_BACKWARD;
75087513
n->howMany = 1;
7514+
n->location = -1;
75097515
$$ = (Node *) n;
75107516
}
75117517
| FIRST_P opt_from_in cursor_name
@@ -7515,6 +7521,7 @@ fetch_args: cursor_name
75157521
n->portalname = $3;
75167522
n->direction = FETCH_ABSOLUTE;
75177523
n->howMany = 1;
7524+
n->location = -1;
75187525
$$ = (Node *) n;
75197526
}
75207527
| LAST_P opt_from_in cursor_name
@@ -7524,6 +7531,7 @@ fetch_args: cursor_name
75247531
n->portalname = $3;
75257532
n->direction = FETCH_ABSOLUTE;
75267533
n->howMany = -1;
7534+
n->location = -1;
75277535
$$ = (Node *) n;
75287536
}
75297537
| ABSOLUTE_P SignedIconst opt_from_in cursor_name
@@ -7533,6 +7541,7 @@ fetch_args: cursor_name
75337541
n->portalname = $4;
75347542
n->direction = FETCH_ABSOLUTE;
75357543
n->howMany = $2;
7544+
n->location = @2;
75367545
$$ = (Node *) n;
75377546
}
75387547
| RELATIVE_P SignedIconst opt_from_in cursor_name
@@ -7542,6 +7551,7 @@ fetch_args: cursor_name
75427551
n->portalname = $4;
75437552
n->direction = FETCH_RELATIVE;
75447553
n->howMany = $2;
7554+
n->location = @2;
75457555
$$ = (Node *) n;
75467556
}
75477557
| SignedIconst opt_from_in cursor_name
@@ -7551,6 +7561,7 @@ fetch_args: cursor_name
75517561
n->portalname = $3;
75527562
n->direction = FETCH_FORWARD;
75537563
n->howMany = $1;
7564+
n->location = @1;
75547565
$$ = (Node *) n;
75557566
}
75567567
| ALL opt_from_in cursor_name
@@ -7560,6 +7571,7 @@ fetch_args: cursor_name
75607571
n->portalname = $3;
75617572
n->direction = FETCH_FORWARD;
75627573
n->howMany = FETCH_ALL;
7574+
n->location = -1;
75637575
$$ = (Node *) n;
75647576
}
75657577
| FORWARD opt_from_in cursor_name
@@ -7569,6 +7581,7 @@ fetch_args: cursor_name
75697581
n->portalname = $3;
75707582
n->direction = FETCH_FORWARD;
75717583
n->howMany = 1;
7584+
n->location = -1;
75727585
$$ = (Node *) n;
75737586
}
75747587
| FORWARD SignedIconst opt_from_in cursor_name
@@ -7578,6 +7591,7 @@ fetch_args: cursor_name
75787591
n->portalname = $4;
75797592
n->direction = FETCH_FORWARD;
75807593
n->howMany = $2;
7594+
n->location = @2;
75817595
$$ = (Node *) n;
75827596
}
75837597
| FORWARD ALL opt_from_in cursor_name
@@ -7596,6 +7610,7 @@ fetch_args: cursor_name
75967610
n->portalname = $3;
75977611
n->direction = FETCH_BACKWARD;
75987612
n->howMany = 1;
7613+
n->location = -1;
75997614
$$ = (Node *) n;
76007615
}
76017616
| BACKWARD SignedIconst opt_from_in cursor_name
@@ -7605,6 +7620,7 @@ fetch_args: cursor_name
76057620
n->portalname = $4;
76067621
n->direction = FETCH_BACKWARD;
76077622
n->howMany = $2;
7623+
n->location = @2;
76087624
$$ = (Node *) n;
76097625
}
76107626
| BACKWARD ALL opt_from_in cursor_name
@@ -7614,6 +7630,7 @@ fetch_args: cursor_name
76147630
n->portalname = $4;
76157631
n->direction = FETCH_BACKWARD;
76167632
n->howMany = FETCH_ALL;
7633+
n->location = -1;
76177634
$$ = (Node *) n;
76187635
}
76197636
;
@@ -12752,6 +12769,7 @@ DeclareCursorStmt: DECLARE cursor_name cursor_options CURSOR opt_hold FOR Select
1275212769
DeclareCursorStmt *n = makeNode(DeclareCursorStmt);
1275312770

1275412771
n->portalname = $2;
12772+
n->location = @2;
1275512773
/* currently we always set FAST_PLAN option */
1275612774
n->options = $3 | $5 | CURSOR_OPT_FAST_PLAN;
1275712775
n->query = $7;

0 commit comments

Comments
 (0)