Re: 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)