Robert Haas <robertmhaas@gmail.com> writes:
> If there is, maybe we should discard the integer IDs and just use
> strings directly. I could make GetExplainExtensionState() and
> SetExplainExtensionState() take const char *extension_name arguments
> instead of int extension_id, and just get rid of GetExplainExtensionId
> entirely. I chose the current design because I thought that the IDs
> did not need to be shared and I thought it would be a bit extravagant
> to have an EXPLAIN extension need to do a hash table lookup for every
> plan node, but maybe it wouldn't really matter in practice.
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.
Other than that point, I think 0001 and 0002 are ready.
> Regarding commit timeframe, you expressed a preference for 0001 and
> 0002 to be committed "soon," but I'm going to be leaving town for a
> week on Saturday and would prefer not to put myself in a situation
> where somebody is expecting me to fix the buildfarm while I am gone.
> So I'm happy to commit them today or tomorrow if you don't mind being
> responsible for anything that blows up while I'm gone, or I'm happy to
> have you commit them whenever you want, but barring one of those
> outcomes, I'm going to deal with it on or after March 17th.
Sure, I will backstop you if you want to push and run. There's
at least one nearby thread where some interest has been expressed in
using this (the postgres_fdw include-the-remote-EXPLAIN business).
So I think there is value in having it in the tree sooner.
regards, tom lane
