From: Robert Haas
>>
>> One thing that needs some thought here is what happens if the value of
>> pgss_enabled() changes. For example we don't want a situation where
>> if the value changes from off to on between one stage of processing
>> and another, the server crashes.
>>
>> I don't know whether that's a risk here or not; it's just something to
>> think about.
This is definitely an important consideration for this change. A hook could
have the implicit assumption that a query ID is always generated.
From: PAscal
> Hi, here is a simple test where I commented that line in pgss_post_parse_analyze
> to force return; (as if pgss_enabled() was disabled) but kept pgss_enabled() every where else
>
> /* Safety check... */
> // if (!pgss || !pgss_hash || !pgss_enabled())
> return;
>
> This works without crash as you can see here after:
In theory, the rest of the hooks look solid.
As mentioned above, I think the major concern would be a hook that depends
on a variable generated in pgss_post_parse_analyze. Two hooks
(pgss_ExecutorStart, pgss_ExecutorEnd) depend on the query ID
generated from pgss_post_parse_analyze. Fortunately, both of these
functions already check for query ID before doing work.
I really appreciate you putting this change into practice.
Great to see your results align with mine.
Thanks Pascal!!!
--
Raymond Martin
ramarti@microsoft.com
Azure Database for PostgreSQL