Re: pg_stat_statements stores WITH HOLD cursor query with literals

> > 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.
>
> This was itching me, so I have looked at what it would look like,
> finishing with the attached set of two patches:
> - One to introduce a backend-side facility to give for extensions a
> way to plug in custom data into QueryDesc.
> - The second patch is a refactored v2, that makes use of the new
> infra.
>
> That feels rather neat. This facility could be extended to other
> things, like auto_explain perhaps?

I spent some time thinking about this today, and I can now see
its usefulness. Initially I was thinking that nesting_level is the only
thing that needs this state in pg_stat_statements and auto_explain, and
was not sure if this complexity is worth it.

But then I found a few cases. I am not sure how prevalent they are in
real-world use, but they seem worth handling.

Take for example a role switch between the DECLARE CURSOR and the cursor
cleanup. Maybe not very common, but perhaps applications dealing with
multi-tenancy/RLS could end up running into this without realizing.

```
CREATE ROLE user1;
CREATE ROLE user2;

SET pg_stat_statements.track = 'all';
SELECT pg_stat_statements_reset();

BEGIN;
SET ROLE user1;
DECLARE c CURSOR FOR SELECT 1 WHERE 1 = 1;
FETCH 1 FROM c;

SET ROLE user2;
COMMIT;

RESET ROLE;

SELECT userid::regrole, toplevel, calls, query
FROM pg_stat_statements
WHERE query LIKE 'DECLARE c%'
ORDER BY userid::regrole::text, toplevel, query;
```

This currently gives me:

```
userid | toplevel | calls | query
--------+----------+-------+----------------------------------------------
user1 | t | 1 | DECLARE c CURSOR FOR SELECT $1 WHERE $2 = $3
user2 | t | 1 | DECLARE c CURSOR FOR SELECT 1 WHERE 1 = 1;
```

So I think pg_stat_statements should save the userid in the QueryDesc
state as well. Otherwise ExecutorEnd() can use a different current user than
the one that started the query.

There is also a similar shape with auto_explain. The cursors were not
sampled when they started. After raising auto_explain.sample_rate, the
later SELECT makes the new sample rate effective. Then COMMIT closes
the cursors while auto_explain thinks the current query should be
sampled.

```
LOAD 'auto_explain';
CREATE EXTENSION IF NOT EXISTS pg_stat_statements;

SET client_min_messages = notice;
SET auto_explain.log_level = notice;
SET auto_explain.log_min_duration = 0;
SET auto_explain.sample_rate = 0;
SET pg_stat_statements.track = 'all';

BEGIN;
DECLARE c1 CURSOR WITH HOLD FOR SELECT 'secret 1';
DECLARE c2 CURSOR WITH HOLD FOR SELECT 'secret 2';
DECLARE c3 CURSOR WITH HOLD FOR SELECT 'secret 3';

SET auto_explain.sample_rate = 1;
SELECT 1;

COMMIT;
```

One could argue which behavior we want to see in the above, but I think
the point is that this QueryDesc state support could help us close some
of these issues.

There is also a smaller question around toplevel. A later change to
pg_stat_statements.track can still affect whether ExecutorEnd() stores
the entry. We could also track the toplevel if we choose to make this
fix also.

So, it is nothing we need to do now, fixing the nesting_level is as is done in
v3- is good to close this thread, and as a follow-up I do think we should
fix the userid one. That one seems quite important as it deals with wrong
userid attribution.

As far as v3- goes, they both look good to me, and I could not find
anything wrong with them.

--
Sami