Re: Proposal - Allow extensions to set a Plan Identifier - Mailing list pgsql-hackers
| From | Lukas Fittl |
|---|---|
| Subject | Re: Proposal - Allow extensions to set a Plan Identifier |
| Date | |
| Msg-id | CAP53Pkw7HD+3sssn5UcASWLn+a9oRJOM9hXteDCg64JJ662bHQ@mail.gmail.com Whole thread Raw |
| In response to | Re: Proposal - Allow extensions to set a Plan Identifier (Michael Paquier <michael@paquier.xyz>) |
| List | pgsql-hackers |
On Tue, Mar 25, 2025 at 12:13 AM Michael Paquier <michael@paquier.xyz> wrote:
On Mon, Mar 24, 2025 at 06:47:59PM -0500, Sami Imseih wrote:
> I know it was mentioned above by both Michael and Andrei that
> AppendJumble should not be exposed. I am not sure I agree with
> that. I think it should be exposed, along with
> JUMBLE_FIELD, JUMBLE_FIELD_SINGLE and JUMBLE_STRING
> and _jumbleList.
>
> I am afraid that if we don't expose these routines/macros, the
> extension will have
> to re-invent those wheels. This is not included in v1-0001, but If
> there is no objection,
> I can update the patch. Thoughts?
Hmm, yes, the macros could prove to be useful to allow extensions to
compile their own code the manipulations they want to do on the node
structures they'd like to touch, but wouldn't that mean that they
would copy in some way a portion of what gen_node_support.pl does?
I agree with Sami that we should expose AppendJumble (it'd be necessary for any extension that wants to re-use the jumbling logic), and I don't see a reason to skip over the convenience macros, but also not hard to recreate those. AFAIK you could get by without having access to _jumbleList, since you can just call JumbleNode which can jumble lists.
The initial patch I had posted over at the other thread [0], (patch 0003 to be precise in the initial set, which was partially based on work Sami had done previously), showed what I think is the minimum we should enable:
case T_IndexScan:
{
IndexScan *splan = (IndexScan *) plan;
...
{
IndexScan *splan = (IndexScan *) plan;
...
JUMBLE_VALUE(splan->indexid);
JumbleNode(jstate, (Node *) splan->indexqual);
...
...
i.e. allow someone to defer to core logic for expressions, but require them to implement their own (manual) way of writing the jumble functions/conditions for the more limited set of expression-containing nodes they care about (like plan nodes).
Of course, thinking about other use cases, I could imagine someone would potentially be interested to change core jumbling logic for only a specific node (e.g. implementing a query ID that considers constant values to be significant), but I'd argue that making that level of customization work is out of scope / hard to do in general.
Perhaps we should think more carefully about this part, and refactor
this script to work as a perl module so as it could be reused by some
external code, at least for the parsing of the headers and the
attributes assigned to each nodes and their attributes?
I've been thinking about how to rework things for a PG18-requiring pg_stat_plans extension I'd like to publish, and if I were to choose a node attribute-based approach I'd probably:
1. Check out the upstream Postgres source for the given version I'm generating jumble code for
2. Modify the headers as needed to have the node attributes I want
3. Call the gen_node_support.pl via the build process
4. Copy out the resulting jumble node funcs/conds
Even with the state currently committed this is already possible, but (4) ends up with a lot of duplicated code in the extension. Assuming we have a way to call jumbleNode + AppendJumble and macros, that step could only keep the jumble conds/funcs that are not defined in core.
Even with the state currently committed this is already possible, but (4) ends up with a lot of duplicated code in the extension. Assuming we have a way to call jumbleNode + AppendJumble and macros, that step could only keep the jumble conds/funcs that are not defined in core.
So based on that workflow I'm not sure turning gen_node_support.pl into a re-usable module would help, but that's just my perspective without having built this out (yet).
Thanks,
Lukas
--
Lukas Fittl
pgsql-hackers by date: