Re: making EXPLAIN extensible

On Thu, Mar 6, 2025 at 4:24 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Mar 6, 2025 at 4:16 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > I find a good deal of attraction in getting rid of the IDs and
> > just using names. Nor do I believe we need a hash table.
> > (1) Surely there will not be so many extensions using this within a
> > single EXPLAIN that a simple loop with strcmp()'s isn't good enough.
> > (2) The IDs aren't free either; where will an extension keep the
> > ID it assigned? We're trying to move away from global variables.
> >
> > But, if you're convinced otherwise, the current design is OK.
>
> Interesting. I hadn't even considered just iterating every time to
> find the ID, but I agree with you that might be totally fine. As you
> say, we're not expecting there to be many extensions here. I can try
> coding that up and see how it looks (or you can, if you like).

Here's v6, doing it that way. I found that the simplest thing to do
was just push the call to GetExplainExtensionId() inside
Get/SetExplainExtensionState(). With this approach, the backend-scope
IDs still exist, but they are private to explain_state.c. An alternate
design could be to make each individual ExplainState have its own list
of extension names alongside its own list of opaque pointers, so that
the IDs become ExplainState-scoped rather than backend-scoped. At the
moment, that seems to me to be just deciding to make the code more
complicated for no obvious benefit, but maybe I'm missing something.
At any rate, my overall conclusion here is that this is giving up a
probably-insignificant amount of performance for an
also-not-terribly-significant abstraction improvement, so I find it a
little hard to get excited about it one way or the other, but it's
fine.

Tom, what do you think?

Thanks,

--
Robert Haas
EDB: http://www.enterprisedb.com