Re: pg_stat_statements: a reset mid-query leaves an entry unnormalized forever

Hi,

> If pg_stat_statements_reset() lands between a statement's parse
> analysis and its ExecutorEnd, the entry is recreated from the raw
> source text and keeps those literals for every later execution of that
> query. It does not recover on its own.
>
> Repro:
>
> SELECT pg_stat_statements_reset();
> SELECT 1234 FROM
> (SELECT CASE WHEN 1=1 THEN pg_stat_statements_reset() END) s;
> SELECT 5678 FROM
> (SELECT CASE WHEN 1=0 THEN pg_stat_statements_reset() END) s;
> SELECT 9999 FROM
> (SELECT CASE WHEN 1=0 THEN pg_stat_statements_reset() END) s;
>
> SELECT calls, query
> FROM pg_stat_statements
> WHERE query LIKE '%CASE%';
>
> calls | query
> -------+----------------------------------------------
> 3 | SELECT 1234 FROM (SELECT CASE WHEN 1=1 THEN
> | pg_stat_statements_reset() END) s

Yes, in this case, the statement is initially stored normalized during
parse because that is the only time JumbleState and constant locations
are available. Then mid-query, pg_stat_statements_reset() removes the
normalized query string, and in ExecutorEnd the query is stored again,
but this time without normalization since JumbleState is not available
at that point.

> I think it is worth another look because the bad text is durable rather
> than a single bad sample. Tom's second suggestion there still looks
> like the right fix: discard the execution stats when the entry is
> gone, rather than store literals.

-1 to discarding stats in that case. The purpose of this extension is to
provide stats first. Normalization is not guaranteed.

> How ExecutorEnd recognises that case with jstate already gone is the
> open question, and I am happy to write a patch if the list has a
> preferred direction.

One option would be to carry enough jumbling state through to
ExecutorEnd so the replacement entry can still be normalized, but that
may be too expensive, especially for short queries with large constants.
I have not benchmarked that.

> Either way, pgstatstatements.sgml still does not mention that
> normalization is best-effort, which is where that thread landed.

The doc says "Queries on which normalization can be applied may
be observed with constant values in pg_stat_statements, especially
when there is a high rate of entry deallocations", so there is no
guarantee that a caller will always get normalized text.

Query text such as DO blocks don't get normalized already, even
in the normal use case. The body of the block gets treated it
like a string, so it does does not get normalized.

```
DO $$
BEGIN
PERFORM a FROM generate_series(1,10) AS gs(a) WHERE a = 3;
PERFORM a FROM generate_series(1,10) AS gs(a) WHERE a = 7;
END
$$ LANGUAGE plpgsql;

query
--------------------------------------------------------------------
select query from pg_stat_statements
DO $$ +
BEGIN +
PERFORM a FROM generate_series(1,10) AS gs(a) WHERE a = 3;+
PERFORM a FROM generate_series(1,10) AS gs(a) WHERE a = 7;+
END +
$$ LANGUAGE plpgsql
(2 rows)
```

There are also other cases I am not thinking of in which normalization
is not guaranteed even in the regular usage case.

That said, the docs could perhaps be made a bit clearer here and call
out this sort of behavior more explicitly.

--
Sami Imseih
Amazon Web Services (AWS)