Re: Proposal - Allow extensions to set a Plan Identifier
Lukas Fittl <lukas@fittl.com>
From: Lukas Fittl <lukas@fittl.com>
To: Sami Imseih <samimseih@gmail.com>, David Rowley <dgrowleyml@gmail.com>, Michael Paquier <michael@paquier.xyz>
Cc: Andrei Lepikhov <lepihov@gmail.com>,
Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2025-04-08T01:11:06Z
Lists: pgsql-hackers
Attachments
- v4-0001-Allow-plugins-to-Jumble-an-expression.patch (application/x-patch)
Whilst rebasing the pg_stat_plans work on top of this, I realized that we
should probably make this a conditional instead of an assert - if you are
jumbling a tree that contains expressions you'd want to be able to jumble a
node that has a location (but don't record it). Attached v4 that modifies
it like that.
However, since we're basically at feature freeze (and it seems unlikely
this gets into 18), I have a quick alternate proposal:
What if, for 18, we just make DoJumble exported to be used by plugins?
DoJumble was added in David's fix for NULL handling in f31aad9b0, but
remained local to queryjumblefuncs.c. Exporting that would enable an
extension to jumble expression fields contained in plan nodes and get the
hash value, and then do its own hashing/jumbling of the plan nodes as it
sees fit, without reinventing the wheel. I'll be around the next hours to
make a quick patch (though its basically just two lines) if this is of
interest.
Thanks,
Lukas
On Wed, Apr 2, 2025 at 12:43 PM Sami Imseih <samimseih@gmail.com> wrote:
> > I reviewed the patch and I only have one comment. I still think
> > we need an Assert inside RecordConstantLocation to make sure clocations
> > is allocated if the caller intends to record constant locations.
> >
> > RecordConstLocation(JumbleState *jstate, int location, bool squashed)
> > {
> > + Assert(jstate->clocations);
> > +
>
> Here is v3 with the Assert in place
>
>
> --
> Sami Imseih
> Amazon Web Services (AWS)
>
--
Lukas Fittl