Re: Generating code for query jumbling through gen_node_support.pl

On 18.01.23 08:04, Michael Paquier wrote:
> On Tue, Jan 17, 2023 at 04:52:28PM +0900, Michael Paquier wrote:
>> On Tue, Jan 17, 2023 at 08:43:44AM +0100, Peter Eisentraut wrote:
>>> Ok, I understand now, and I agree with this approach over the opposite. I
>>> was confused because the snippet you showed above used "jumble_ignore", but
>>> your patch is correct as it uses "jumble_location".
>>
>> Okay. I'll refresh the patch set so as we have only "jumble_ignore",
>> then, like v1, with preparatory patches for what you mentioned and
>> anything that comes into mind.
>
> This is done as of the patch series v3 attached:
> - 0001 reformats all the comments of the nodes.
> - 0002 moves the current files for query jumble as of queryjumble.c ->
> queryjumblefuncs.c and utils/queryjumble.h -> nodes/queryjumble.h.
> - 0003 is the core feature, where I have done a second pass over the
> nodes to make sure that things map with HEAD, incorporating the extra
> docs coming from v2, adding a bit more.

This patch structure looks good.

>>> That said, the term "jumble" is really weird, because in the sense that we
>>> are using it here it means, approximately, "to mix together", "to unify".
>>> So what we are doing with the Const nodes is really to *not* jumble the
>>> location, but for all other node types we are jumbling the location. At
>>> least that is my understanding.
>>
>> I am quite familiar with this term, FWIW. That's what we've inherited
>> from the days where this has been introduced in pg_stat_statements.
>
> I have renamed the node attributes to query_jumble_ignore and
> no_query_jumble at the end, after considering Peter's point that only
> "jumble" could be fuzzy here. The file names are changed in
> consequence.

I see that in the 0003 patch, most location fields now have an explicit
markup with query_jumble_ignore. I thought we had previously resolved
to consider location fields to be automatically ignored unless
explicitly included (like for the Const node). This appears to invert
that? Am I missing something?