Re: [PATCH] Optionally record Plan IDs to track plan changes for a query

> This fixes the long comments in plannodes.h to make it easier to add the
> attribute annotation. It made the most sense to make this the first patch
> in the set.

> A commit that happened last Friday made also this to have conflict.

Thanks for committing v5-0001. I am not sure why there is comment
that is not correctly indented. Attached is a fix for that


> I don't understand why there is a need for publishing AppendJumble()
> while it remains statis in jumblefuncs.c.  This is not needed in 0003
> and 0004, either.

In v5-0002, AppendJumble is no longer static.

-static void
+void
 AppendJumble(JumbleState *jstate, const unsigned char *item, Size size)
 {

Maybe I am missing something?

> Should we use more generic names for the existing custom_query_jumble,
> no_query_jumble, query_jumble_ignore and query_jumble_location?  Last
> time I've talked about that with Peter E, "jumble" felt too generic,
> so perhaps we're looking for a completely new term?  This impacts as
> well the naming of the existing queryjumblefuncs.c.  The simplest term
> that may be possible here is "hash", actually, because that's what we

> Point is that query_jumble_ignore is used in the planner nodes, which
> feels inconsistent, so perhaps we could rename query_jumble_ignore and
> no_query_jumble to "hash_ignore" and/or "no_hash", or something like
> that?  This may point towards the need of a split, not sure, still the
> picture is incomplete.

I was thinking about this as I was reworking the comments in jumblefuncs.c
for v5-0002.

I am OK with moving away from "jumble" in-lieu of something else, but
my thoughts are we should actually call this process "fingerprint"
( a term we already use in the queryjumblefuncs.c comment ).
A fingerprint consists of all the interesting parts of a node tree that are
appended and the final product is a hash of this fingerprint ( i.e. queryId )
For node attributes we can specify "fingerprint_ignore"
or "no_fingerprint". What do you think?

> The concept of location does not apply to plans, based on the
> current proposal, so perhaps we should talk about "query normalization
> location"?

Are you referring to JUMBLE_LOCATION? and whether to keep it in
queryjumblefuncs.c ( or jumblefuncs.c as is being proposed )?


Regards,

Sami