Re: [POC] Allow an extension to add data into Query and PlannedStmt nodes - Mailing list pgsql-hackers

Hi,

On Wed, Mar 29, 2023 at 12:02:30PM +0500, Andrey Lepikhov wrote:
>
> Previously, we read int this mailing list some controversial opinions on
> queryid generation and Jumbling technique. Here we don't intend to solve
> these problems but help an extension at least don't conflict with others on
> the queryId value.
>
> Extensions could need to pass some query-related data through all stages of
> the query planning and execution. As a trivial example, pg_stat_statements
> uses queryid at the end of execution to save some statistics. One more
> reason - extensions now conflict on queryid value and the logic of its
> changing. With this patch, it can be managed.

I just had a quick lookc at the patch, and IIUC it doesn't really help on that
side, as there's still a single official "queryid" that's computed, stored
everywhere and later used by pg_stat_statements (which does then store in
additionally to that official queryid).

You can currently change the main jumbling algorithm with a custom extension,
and all extensions will then use it as the source of truth, but I guess that
what you want is to e.g. have an additional and semantically different queryid,
and create multiple ecosystem of extensions, each using one or the other source
of queryid without changing the other ecosystem behavior.
>
> This patch introduces the structure 'ExtensionData' which allows to manage
> of a list of entries with a couple of interface functions
> addExtensionDataToNode() and GetExtensionData(). Keep in mind the possible
> future hiding of this structure from the public interface.
> An extension should invent a symbolic key to identify its data. It may
> invent as many additional keys as it wants but the best option here - is no
> more than one entry for each extension.
> Usage of this machinery is demonstrated by the pg_stat_statements example -
> here we introduced Bigint node just for natively storing of queryId value.
>
> Ruthless pgbench benchmark shows that we got some overhead:
> 1.6% - in default mode
> 4% - in prepared mode
> ~0.1% in extended mode.

That's a quite significant overhead.  But the only reason to accept such a
change is to actually use it to store additional data, so it would be
interesting to see what the overhead is like once you store at least 2
different values there.

> - Are we need to invent a registration procedure to do away with the names
> of entries and use some compact integer IDs?

Note that the patch as proposed doesn't have any defense for two extensions
trying to register something with the same name, or update a stored value, as
AddExtensionDataToNode() simply prepend the new value to the list.  You can
actually update the value by just storing the new value, but it will add a
significant overhead to every other extension that want to read another value.

The API is also quite limited as each stored value has a single identifier.
What if your extension needs to store multiple things?  Since it's all based on
Node you can't really store some custom struct, so you probably have to end up
with things like "my_extension.my_val1", "my_extension.my_val2" which isn't
great.