pg_stat_statements stores WITH HOLD cursor query with literals

Hi,

Without auto_explain, only the first row appears. Without auto_explain
but with pg_stat_statements.track = 'all', the second row appears too.

The cursor's query starts inside DECLARE (nesting level 1), but a
WITH HOLD cursor finishes it at commit (nesting level 0).
pgss_ExecutorEnd stores any query that has a queryId and query_instr,
even when pg_stat_statements did not request query_instr itself or
requested it at another nesting level. It then finds no normalized
entry, so it stores the raw text.

This was also discussed in https://commitfest.postgresql.org/patch/6342/.

The core issue is that nesting level is currently backend-local,
while a QueryDesc can outlive the executor/utility nesting level at which it
was created. WITH HOLD cursors are one example: the cursor query starts
inside DECLARE, but can finish later when the cursor is implicitly closed.

I got hung up in the earlier thread because I was not sure whether the best
path was for core to track nesting_level directly, or to let extensions attach
their own nesting level to a QueryDesc. I now think the latter is the smaller
and safer fix.

The patch I am testing adds a nesting_level field to QueryDesc, initialized to
-1, and exposes a small API so extensions can set it once:

static inline void
SetQueryDescNestingLevel(QueryDesc *queryDesc, int nesting_level)
{
if (queryDesc->nesting_level == -1)
queryDesc->nesting_level = nesting_level;
}

pg_stat_statements then records the nesting level when it requests query-level
instrumentation, and later uses the saved QueryDesc level in ExecutorEnd
instead of the current backend-local level. auto_explain can also set the
same field when it requests query-level instrumentation; the helper preserves
the first value if multiple extensions are loaded.

Adding Michael for his thoughts on this also.

--
Sami Imseih
Amazon Web Services (AWS)

Hi Sami,

Thanks for the pointer to the earlier thread.

One concern with a single shared field: extensions count nesting
differently. auto_explain has no ProcessUtility hook, so when DECLARE
starts the cursor's query, auto_explain is at level 0 while
pg_stat_statements is at level 1.

In my first repro (track = top), pg_stat_statements does not request
instrumentation at level 1, so only auto_explain sets the field, to 0.
pg_stat_statements would then read 0 in ExecutorEnd and still store
the cursor query as top level. With track = 'all' both set it, and
which value wins depends on which extension's hook runs first.

So pg_stat_statements probably needs to know whether it set the level
itself, and skip the query when it did not. Would a per-extension slot,
or a single nesting level tracked by core, work better here?

Thanks,
Gaurav Singh

On Tue, Oct 13, 2026 at 3:20 AM Sami Imseih <samimseih.pg@gmail.com> wrote:

One concern with a single shared field: extensions count nesting
differently. auto_explain has no ProcessUtility hook, so when DECLARE
starts the cursor's query, auto_explain is at level 0 while
pg_stat_statements is at level 1.

In my first repro (track = top), pg_stat_statements does not request
instrumentation at level 1, so only auto_explain sets the field, to 0.
pg_stat_statements would then read 0 in ExecutorEnd and still store
the cursor query as top level. With track = 'all' both set it, and
which value wins depends on which extension's hook runs first.

Good point, a single field can't hold two different ideas of what the
nesting level is.

So pg_stat_statements probably needs to know whether it set the level
itself, and skip the query when it did not. Would a per-extension slot,
or a single nesting level tracked by core, work better here?

per-extension slot sounds a bit too heavy-handed for me. And I'd rather
not have core track the nesting level either, since core would have to
pick one definition for every extension, and extensions can
legitimately disagree on what counts as nesting.

So the attached v1 goes the simpler way. It adds a field called
start_nesting_level, which can be set at ExecutorStart by any extension
that counts nesting levels the way pg_stat_statements does. The comment
on the field spells out what that means, and an extension that counts
differently, like auto_explain, should just not set it, so v1 doesn't
change auto_explain at all.

There is precedent for this kind of field. Query.queryId and
PlannedStmt.planId are both core fields that "can be set by plugins",
and it's up to the plugins that set them to agree on what they mean.

Since every extension that sets it computes the same value, it doesn't
matter who sets it or in what order. And in your track = top case,
nobody sets the field, so pg_stat_statements skips the query.

v1 is a sketch of that without any tests yet. I will add if there is
agreement on
this being the way forward. This seems like the most straightforward way.
Maybe others have a different idea.

--
Sami Imseih
Amazon Web Services (AWS)

v1 is a sketch of that without any tests yet. I will add if there is
agreement on
this being the way forward. This seems like the most straightforward way.
Maybe others have a different idea.

Attached v2 is cleaned up with a test.

--
Sami Imseih
Amazon Web Services (AWS)

LGTM! Thank you for the patch.

Warm Regards,
Gaurav Singh

On Wed, Oct 14, 2026 at 7:33 AM Sami Imseih <samimseih.pg@gmail.com> wrote:

Thanks. I modified [1]https://commitfest.postgresql.org/patch/6342/ to link to this thread now instead of [2]/messages/by-id/CAA5RZ0v281uwSudcFrY8BymSRDM4J6PAHfzqg_9R+i7zWbTb2A@mail.gmail.com

[1]: https://commitfest.postgresql.org/patch/6342/
[2]: /messages/by-id/CAA5RZ0v281uwSudcFrY8BymSRDM4J6PAHfzqg_9R+i7zWbTb2A@mail.gmail.com

--
Sami Imseih
Amazon Web Services (AWS)

I think the issue has existed since PG18, could we have a backpatch too?

Thanks,
Gaurav Singh

On Wed, Oct 14, 2026 at 8:11 PM Sami Imseih <samimseih.pg@gmail.com> wrote:

On Thu, Oct 15, 2026 at 10:36:06AM +0530, Gaurav Singh wrote:

I think the issue has existed since PG18, could we have a backpatch too?

With a change in QueryDesc? I'm afraid not.

+	/*
+	 * Nesting level at which this query started, or -1 if not set.  An
+	 * extension may set this from its ExecutorStart hook and read it back at
+	 * ExecutorEnd, which can run at a different level.  For example, a
+	 * cursor's query starts inside DECLARE but may end at COMMIT.
+	 *
+	 * The level counts as pg_stat_statements does: 0 for a statement sent by
+	 * the client, including EXECUTE of a prepared statement; one deeper for
+	 * each query run by another statement or by a function.  All extensions
+	 * that set this field must use that definition; one that counts
+	 * differently must leave it alone.
+	 */
+	int			start_nesting_level;

This is a one-time extension field, tied exclusively to PGSS that may
or may not conflict depending on what other extensions do. This does
not give me good vibes in terms of extensibility, due to the easy
conflicts with other extensions.

We have a similar concept for ExplainState with its extension_state
indexed by GetExplainExtensionId() based on an extension name.
Perhaps we should do something similar for QueryDesc? That would take
care of the conflicting states across extensions at least, giving
extensions way to store in the QueryDesc what they want. Here, that's
the nesting_level that you want to track *only* in PGSS and have no
other extensions meddle with it.
--
Michael