Looks like some emails were sent before I could send my draft email, but hopefully this should make the follow up work easier :)
Attached a v4 patch set with a few minor changes to plan ID jumbling:
*
Range table jumbling is now done in a separate JumbleRangeTable
function after setrefs.c walked the tree - this way we avoid having
custom logic for RT Indexes in the node jumbling, and keeping a reference to PlannerGlobal in the jumble struct
* Moved the
JumbleNode call to the bottom of the set_plan_references function for
clarity - previously it was before descending into inner/outer plan, but
after some other recursive calls to set_plan_references, which didn't
really make sense
* Fixed a bug with JUMBLE_ARRAY incorrectly
taking the reference of the array (which caused planid to change
incorrectly between runs)
* Added JUMBLE_BITMAPSET
Further, I've
significantly reduced the number of fields ignored for plan jumbling:
Basically the approach taken in this version is that only things that
would negatively affect the planid (i.e. make it unique when it
shouldn't be) are ignored, vs ignoring duplicate fields and fields that
are only used by the executor (which is what v1-v3 did). I'm not 100% sure that's the
right approach (but it does keep the diff a good amount smaller), I think the tradeoff here is basically jumbling
performance vs maintenance overhead when fields are added/changed.
This does not yet move field-specific comments to
their own line in nodes where we're adding node attributes, I'll leave that for Sami to work on.
>> Separately I've been thinking how we could best have a discussion/review on
>> whether the jumbling of specific plan struct fields is correct. I was
>> thinking maybe a quick wiki page could be helpful, noting why to jumble/not
>> jumble certain fields?
> Makes sense. This is a complicated topic.
+1 for the Wiki page
Thanks,
Lukas