On Tue, May 20, 2025 at 08:38:47AM +0900, Michael Paquier wrote: > With the semicolon in place, stmt_len gets set for the last query of > the string. Still digging more..
And got it. The problem is that we are failing to update the statement location in a couple of cases with subqueries, and that we should handle (p_stmt_len == 0) as of using the remaining bytes in the string when a location is available, but the code was too aggressive in thinking that the length = 0 case should be always discarded. Once I have fixed that, I've been a bit puzzled by the difference in output in the tests of pg_overexplain, but I think that the new output is actually the correct one: the queries whose plan outputs have changed are passed as arguments of the explain_filter() function, hence the location of the inner queries point at the start location of the inner query instead of the start of the top-level query. Note that if you add a semicolon at the end of these three queries in the pg_overexplain tests, we finish with an end location reported. I have also played with 499edb0 reverted and noted that the results of pg_overexplain were inconsistent when the module has been originally introduced, with two queries choking a bit. -- Michael
From 74b484ee4b05b5ff7139bfc3ae1c194060a6dfaf Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 20 May 2025 12:53:35 +0900
Subject: [PATCH] Fix regression with statement location of some subqueries
Note the changes in the output of pg_overexplain, as the new locations
refer to the beginning of the argument passed to the function
explain_filter().
Blah.
---
src/backend/parser/analyze.c | 20 +++----
.../expected/pg_overexplain.out | 8 +--
.../expected/level_tracking.out | 57 +++++++++++++++++++
.../pg_stat_statements/sql/level_tracking.sql | 32 +++++++++++
4 files changed, 102 insertions(+), 15 deletions(-)
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 1f4d6adda524..bc524f0754ae 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -260,13 +260,6 @@ setQueryLocationAndLength(ParseState *pstate, Query *qry, Node *parseTree)
{
ParseLoc stmt_len = 0;
- /*
- * If there is no information about the top RawStmt's length, leave it at
- * 0 to use the whole string.
- */
- if (pstate->p_stmt_len == 0)
- return;
-
switch (nodeTag(parseTree))
{
case T_InsertStmt:
@@ -308,16 +301,21 @@ setQueryLocationAndLength(ParseState *pstate, Query *qry, Node *parseTree)
/* Statement's length is known, use it */
qry->stmt_len = stmt_len;
}
- else
+ else if (pstate->p_stmt_len > 0)
{
/*
- * Compute the statement's length from the statement's location and
- * the RawStmt's length and location.
+ * The top RawStmt's length is known, so calculate the statement's
+ * length from the statement's location and the RawStmt's length and
+ * location.
*/
qry->stmt_len = pstate->p_stmt_len - (qry->stmt_location - pstate->p_stmt_location);
}
- /* The calculated statement length should be calculated as positive. */
+ /*
+ * The calculated statement length should be calculated as positive. It
+ * may be possible that stmt_len is 0, in which case the full remaining
+ * string is used.
+ */
Assert(qry->stmt_len >= 0);
}
diff --git a/contrib/pg_overexplain/expected/pg_overexplain.out b/contrib/pg_overexplain/expected/pg_overexplain.out
index 44120c388af5..cb5c396c5192 100644
--- a/contrib/pg_overexplain/expected/pg_overexplain.out
+++ b/contrib/pg_overexplain/expected/pg_overexplain.out
@@ -119,7 +119,7 @@ $$);
Subplans Needing Rewind: none
Relation OIDs: NNN...
Executor Parameter Types: none
- Parse Location: 0 to end
+ Parse Location: 41 to end
RTI 1 (relation, inherited, in-from-clause):
Eref: vegetables (id, name, genus)
Relation: vegetables
@@ -240,7 +240,7 @@ $$);
<Subplans-Needing-Rewind>none</Subplans-Needing-Rewind> +
<Relation-OIDs>NNN...</Relation-OIDs> +
<Executor-Parameter-Types>none</Executor-Parameter-Types> +
- <Parse-Location>0 to end</Parse-Location> +
+ <Parse-Location>53 to end</Parse-Location> +
</PlannedStmt> +
<Range-Table> +
<Range-Table-Entry> +
@@ -344,7 +344,7 @@ $$);
Subplans Needing Rewind: none
Relation OIDs: NNN...
Executor Parameter Types: none
- Parse Location: 0 to end
+ Parse Location: 28 to end
(37 rows)
SET debug_parallel_query = false;
@@ -372,7 +372,7 @@ $$);
Subplans Needing Rewind: none
Relation OIDs: NNN...
Executor Parameter Types: 0
- Parse Location: 0 to end
+ Parse Location: 28 to end
(15 rows)
-- Create an index, and then attempt to force a nested loop with inner index
diff --git a/contrib/pg_stat_statements/expected/level_tracking.out b/contrib/pg_stat_statements/expected/level_tracking.out
index 03bea14d5da0..ee6dea3a9cd3 100644
--- a/contrib/pg_stat_statements/expected/level_tracking.out
+++ b/contrib/pg_stat_statements/expected/level_tracking.out
@@ -1319,6 +1319,63 @@ SELECT toplevel, calls, query FROM pg_stat_statements
t | 1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
(4 rows)
+-- DO block --- multiple inner queries with separators
+SET pg_stat_statements.track = 'all';
+SET pg_stat_statements.track_utility = TRUE;
+CREATE TABLE pgss_do_util_tab_1 (a int);
+CREATE TABLE pgss_do_util_tab_2 (a int);
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t
+---
+ t
+(1 row)
+
+DO $$
+DECLARE BEGIN
+ EXECUTE 'CREATE TABLE pgss_do_table (weird_name INT); DROP TABLE pgss_do_table';
+ EXECUTE 'SELECT a FROM pgss_do_util_tab_1; SELECT a FROM pgss_do_util_tab_2';
+END $$;
+DO $$
+DECLARE BEGIN
+END $$;
+SELECT toplevel, calls, rows, query FROM pg_stat_statements
+ WHERE toplevel IS FALSE
+ ORDER BY query COLLATE "C";
+ toplevel | calls | rows | query
+----------+-------+------+---------------------------------------------
+ f | 1 | 0 | CREATE TABLE pgss_do_table (weird_name INT)
+ f | 1 | 0 | DROP TABLE pgss_do_table
+ f | 1 | 0 | SELECT a FROM pgss_do_util_tab_1
+ f | 1 | 0 | SELECT a FROM pgss_do_util_tab_2
+(4 rows)
+
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+ t
+---
+ t
+(1 row)
+
+-- Note the extra semicolon at the end of the query.
+DO $$
+DECLARE BEGIN
+ EXECUTE 'CREATE TABLE pgss_do_table (weird_name INT); DROP TABLE pgss_do_table;';
+ EXECUTE 'SELECT a FROM pgss_do_util_tab_1; SELECT a FROM pgss_do_util_tab_2;';
+END $$;
+DO $$
+DECLARE BEGIN
+END $$;
+SELECT toplevel, calls, rows, query FROM pg_stat_statements
+ WHERE toplevel IS FALSE
+ ORDER BY query COLLATE "C";
+ toplevel | calls | rows | query
+----------+-------+------+---------------------------------------------
+ f | 1 | 0 | CREATE TABLE pgss_do_table (weird_name INT)
+ f | 1 | 0 | DROP TABLE pgss_do_table
+ f | 1 | 0 | SELECT a FROM pgss_do_util_tab_1
+ f | 1 | 0 | SELECT a FROM pgss_do_util_tab_2
+(4 rows)
+
+DROP TABLE pgss_do_util_tab_1, pgss_do_util_tab_2;
-- PL/pgSQL function - top-level tracking.
SET pg_stat_statements.track = 'top';
SET pg_stat_statements.track_utility = FALSE;
diff --git a/contrib/pg_stat_statements/sql/level_tracking.sql b/contrib/pg_stat_statements/sql/level_tracking.sql
index 6b81230f1869..553059816915 100644
--- a/contrib/pg_stat_statements/sql/level_tracking.sql
+++ b/contrib/pg_stat_statements/sql/level_tracking.sql
@@ -334,6 +334,38 @@ END; $$;
SELECT toplevel, calls, query FROM pg_stat_statements
ORDER BY query COLLATE "C", toplevel;
+-- DO block --- multiple inner queries with separators
+SET pg_stat_statements.track = 'all';
+SET pg_stat_statements.track_utility = TRUE;
+CREATE TABLE pgss_do_util_tab_1 (a int);
+CREATE TABLE pgss_do_util_tab_2 (a int);
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+DO $$
+DECLARE BEGIN
+ EXECUTE 'CREATE TABLE pgss_do_table (weird_name INT); DROP TABLE pgss_do_table';
+ EXECUTE 'SELECT a FROM pgss_do_util_tab_1; SELECT a FROM pgss_do_util_tab_2';
+END $$;
+DO $$
+DECLARE BEGIN
+END $$;
+SELECT toplevel, calls, rows, query FROM pg_stat_statements
+ WHERE toplevel IS FALSE
+ ORDER BY query COLLATE "C";
+SELECT pg_stat_statements_reset() IS NOT NULL AS t;
+-- Note the extra semicolon at the end of the query.
+DO $$
+DECLARE BEGIN
+ EXECUTE 'CREATE TABLE pgss_do_table (weird_name INT); DROP TABLE pgss_do_table;';
+ EXECUTE 'SELECT a FROM pgss_do_util_tab_1; SELECT a FROM pgss_do_util_tab_2;';
+END $$;
+DO $$
+DECLARE BEGIN
+END $$;
+SELECT toplevel, calls, rows, query FROM pg_stat_statements
+ WHERE toplevel IS FALSE
+ ORDER BY query COLLATE "C";
+DROP TABLE pgss_do_util_tab_1, pgss_do_util_tab_2;
+
-- PL/pgSQL function - top-level tracking.
SET pg_stat_statements.track = 'top';
SET pg_stat_statements.track_utility = FALSE;
--
2.49.0
signature.asc
Description: PGP signature
