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