From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Cc: Drouvot, Bertrand <bdrouvot@amazon.com>
Subject: Re: Generating code for query jumbling through gen_node_support.pl
Date: Thu, 19 Jan 2023 09:42:03 +0100
Message-ID: <50b571dc-e0c0-22be-9803-4a3480d2e792@enterprisedb.com> (raw)
In-Reply-To: <Y8eZ9/3STyhF57RV@paquier.xyz>
References: <Y5BHOUhX3zTH/ig6@paquier.xyz>
<34491a4e-2664-6ea7-662c-b13d18d2a92d@enterprisedb.com>
<Y8EOUuViEL8Ewvf/@paquier.xyz>
<d74d2681-5c07-2f6e-d177-995905015311@enterprisedb.com>
<Y8YakMwx2a6Y63oR@paquier.xyz>
<e4801c28-cb5b-3b81-bda1-03af8df1fef9@enterprisedb.com>
<Y8ZTvD6nEqNttAHJ@paquier.xyz>
<Y8eZ9/3STyhF57RV@paquier.xyz>
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?
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@enterprisedb.com, michael@paquier.xyz, pgsql-hackers@lists.postgresql.org, bdrouvot@amazon.com
Subject: Re: Generating code for query jumbling through gen_node_support.pl
In-Reply-To: <50b571dc-e0c0-22be-9803-4a3480d2e792@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox