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
> 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)
--
