Обсуждение: pg_stat_statements: Fix nested tracking for implicitly closed cursors

Hi,

It was brought to my attention that there is pg_stat_statements
behavior where an implicitly closed cursor, via COMMIT or END,
is stored as toplevel for both the utility DECLARE CURSOR
statement and the underlying query.

```
BEGIN;
DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab;
FETCH FORWARD 1 FROM foocur;
 x
---
(0 rows)

COMMIT;
SELECT toplevel, calls, query FROM pg_stat_statements
  ORDER BY query COLLATE "C";
 toplevel | calls |                          query
----------+-------+----------------------------------------------------------
 t        |     1 | BEGIN
 t        |     1 | COMMIT
 t        |     1 | DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab
 t        |     1 | DECLARE FOOCUR CURSOR FOR SELECT * from stats_track_tab;
 t        |     1 | FETCH FORWARD $1 FROM foocur
 t        |     1 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
(6 rows)
```

Also, with track_planning enabled, the underlying query is
stored with toplevel = false for the plans counter and with
toplevel = true for the calls counter, resulting in an
additional entry.

```
SELECT toplevel, calls, plans, query FROM pg_stat_statements
  ORDER BY query COLLATE "C";
 toplevel | calls | plans |                            query
----------+-------+-------+--------------------------------------------------------------
 t        |     1 |     0 | BEGIN
 t        |     1 |     0 | COMMIT
 t        |     1 |     0 | DECLARE FOOCUR CURSOR FOR SELECT * from
stats_track_tab
 t        |     1 |     0 | DECLARE FOOCUR CURSOR FOR SELECT * from
stats_track_tab;
 f        |     0 |     1 | DECLARE FOOCUR CURSOR FOR SELECT * from
stats_track_tab;
 t        |     1 |     0 | FETCH FORWARD $1 FROM foocur
 t        |     1 |     0 | SELECT pg_stat_statements_reset() IS NOT NULL AS t
 t        |     0 |     1 | SELECT toplevel, calls, plans, query FROM
pg_stat_statements+
          |       |       |   ORDER BY query COLLATE "C"
 ```

The reason this occurs is because PortalCleanup, which triggers
ExecutorEnd, runs after the ProcessUtility hook. At that point,
nesting_level has already been reset back to 0.

I am not sure how common this pattern is, but it is probably
worth fixing. At a minimum, we need tests to show this behavior,
but we can do better by checking whether we just processed a
COMMIT statement and setting a flag to let ExecutorEnd increment
nesting_level. There should be no other way to reach ExecutorEnd
after a COMMIT besides PortalCleanup, AFAICT. I could be proven
wrong.

The attached patch fixes this as described above.

Note that, due to f85f6ab051b7, there is a separate issue that
should be improved. Tracking the underlying SQL of a utility
statement using the utility statement itself is confusing
and should be fixed. That is a separate point, but I am
mentioning it here for clarity.


--
Sami Imseih
Amazon Web Services

Hi,

Thanks for the comment!

> Please add a `PG_TRY` and `PG_FINALLY` to make sure we always reset the nesting_level.
>
> Also this will break the following scenario
> ```
> BEGIN;
> COMMIT;
> SELECT 1; -- This will be stored as inner level because COMMIT sets is_txn_end flag
> ```
>
> Can we reset is_txn_end at executorStart to solve the problem?

Correct.Thanks for spotting this oversight. I think the is_txn_end flag must
be reset in all hook functions that call pgss_store, except in pgss_ExecutorEnd,
where we just have to check is_txn_end and increment the nesting_level. Also,
I added a PG_TRY/FINALLY inside pgss_ExecutorEnd to ensure we reset the
nesting_level is reset.

I added a test case for the query you mentioned above.

--
Sami Imseih
Amazon Web Services (AWS)

The patch looks good to me.


--
Martin Huang
Amazon Web Services

On Tue, Jan 13, 2026 at 07:01:48PM -0600, Sami Imseih wrote:
> Correct.Thanks for spotting this oversight. I think the is_txn_end flag must
> be reset in all hook functions that call pgss_store, except in pgss_ExecutorEnd,
> where we just have to check is_txn_end and increment the nesting_level. Also,
> I added a PG_TRY/FINALLY inside pgss_ExecutorEnd to ensure we reset the
> nesting_level is reset.
>
> I added a test case for the query you mentioned above.

@@ -1079,35 +1095,47 @@ pgss_ExecutorEnd(QueryDesc *queryDesc)
 {
     int64        queryId = queryDesc->plannedstmt->queryId;

-    if (queryId != INT64CONST(0) && queryDesc->totaltime &&
-        pgss_enabled(nesting_level))
+    if (is_txn_end)
+        nesting_level++;

Couldn't it be possible that we reach this stack of execution with
is_txn_end = true but we don't intend it to be so?  For example,
imagine first that we reach pgss_ProcessUtility() for a COMMIT
TransactionStmt, second we error without resetting is_txn_end, third a
different portal is executed a with the same backend: we could exit
the executor with is_txn_end set and nesting_level increased but we
did not intend these events to happen.
--
Michael

> Couldn't it be possible that we reach this stack of execution with
> is_txn_end = true but we don't intend it to be so?  For example,
> imagine first that we reach pgss_ProcessUtility() for a COMMIT
> TransactionStmt, second we error without resetting is_txn_end, third a
> different portal is executed a with the same backend: we could exit
> the executor with is_txn_end set and nesting_level increased but we
> did not intend these events to happen.

This is the key point to this fix. So, in v2, is_txn_end is reset at the start
of pgss_planner, pgss_post_parse_analyze and pgss_ProcessUtility. But now
I realized that is_txn_end should also be reset at
pgss_ExecutorStart/Run and should
always be reset at the end of pgss_ExecutorEnd.

So in your example above the new portal will go through one of the hooks which
will reset is_txn_end at the start. Basically, we have to go through
one of pgss_planner,
pgss_post_parse_analyze and pgss_ProcessUtility, pgss_ExecutorRun or
pgss_ExecutorStart
for any command, so resetting this value at the start of these hooks
should ensure that
is_txn_end = true does not incorrectly persist across statements. right?

( oops, I just realized my example at the start of the thread doe not indicate
 that this issue only occurs with pg_stat_statements.track="all" )

--
Sami Imseih
Amazon Web Services (AWS)