Re: [PROPOSAL] Detecting plan changes with plan_id in pg_stat_activity

> AFAICS you're proposing to add an identifier for a specific plan, but no way to
> know what that plan was? How are users supposed to use the information if they
> know something changed but don't know what changed exactly?

I see this as a start to do more useful things with plans.

The patch right out the gate exposes the plan_id in EXPLAIN output
and auto_explain.
This will also be useful for extensions that will provide the plan text.
It is also conceivable that pg_stat_statements can provide an option
To store the plan text?

> - In the POC, the compute_query_id GUC determines if a
> plan_id is to be computed. Should this be a separate GUC?

> Probably, as computing it will likely be quite expensive. Some benchmark on
> various workloads would be needed here.

Yes, more benchmarks will be needed here with more complex plans. I have
Only benchmarked with pgbench at this point.
However, separating this into Its own GUC is what I am leaning towards as well
and will update the patch.

> I only had a quick look at the patch, but I see that you have some code to
> avoid storing the query text multiple times with different planid. How does it
> work exactly, and does it ensure that the query text is only removed once the
> last entry that uses it is removed? It seems that you identify a specific
> query text by queryid, but that seems wrong as collision can (easily?) happen
> in different databases. The real identifier of a query text should be (dbid,
> queryid).

The idea is to lookup the offsets and length of the text in the external file by querid
only. Therefore we can store similar query text for multiple pgss_hash entries
only once.

When a new entry in pgss is not found, the new qtext_hash is consulted to
see if it has information about the offsets/length of the queryid. If found in
qtext_hash, the new pgss_hash entry is created with the offsets found.

If not found in qtext_hash, the query text will be (normalized) and stored in
the external file. Then, a new entry will be created in qtext_hash and
an entry in pgss_hash.

Of course we need to also handle the gc_qtext cleanups, entry_reset, startup
and shutdown scenarios. The patch does this, but I will go back and do more
testing.

> Note that this problem already exists, as the query texts are now stored per
> (userid, dbid, queryid, istoplevel). Maybe this part could be split in a
> different commit as it could already be useful without a planid.

Good point. I will separate this patch.

Regards,

Sami Imseih
Amazon Web Services