Feedback-ID: i131946ab:Fastmail
Message-ID: <50b571dc-e0c0-22be-9803-4a3480d2e792@enterprisedb.com>
Date: Thu, 19 Jan 2023 09:42:03 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.6.1
Subject: Re: Generating code for query jumbling through gen_node_support.pl
Content-Language: en-US
To: Michael Paquier <michael@paquier.xyz>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>,
 "Drouvot, Bertrand" <bdrouvot@amazon.com>
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>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <Y8eZ9/3STyhF57RV@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/50b571dc-e0c0-22be-9803-4a3480d2e792%40enterprisedb.com>
Precedence: bulk

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?