Feedback-ID: i131946ab:Fastmail
Message-ID: <bafce03e-9536-26e4-36af-387ccc2b01ab@enterprisedb.com>
Date: Thu, 26 Jan 2023 09:39:05 +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: <Y8YakMwx2a6Y63oR@paquier.xyz>
 <e4801c28-cb5b-3b81-bda1-03af8df1fef9@enterprisedb.com>
 <Y8ZTvD6nEqNttAHJ@paquier.xyz> <Y8eZ9/3STyhF57RV@paquier.xyz>
 <50b571dc-e0c0-22be-9803-4a3480d2e792@enterprisedb.com>
 <Y8oaA5Vxnr4rzEDJ@paquier.xyz>
 <e4d61d52-4992-d190-b5ad-48c6c752942c@enterprisedb.com>
 <Y8tdmPlzFyNcrPR3@paquier.xyz>
 <bf604c8d-b6e9-284b-327a-cac8417a74e8@enterprisedb.com>
 <Y8+BdCOjxykre5es@paquier.xyz> <Y9By8XDuaLHLNpY0@paquier.xyz>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <Y9By8XDuaLHLNpY0@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/bafce03e-9536-26e4-36af-387ccc2b01ab%40enterprisedb.com>
Precedence: bulk

On 25.01.23 01:08, Michael Paquier wrote:
> On Tue, Jan 24, 2023 at 03:57:56PM +0900, Michael Paquier wrote:
>> Makes sense.  That would be my intention if 0004 is the most
>> acceptable and splitting things makes things a bit easier to review.
> 
> There was a silly mistake in 0004 where the jumbling code relied on
> compute_query_id rather than utility_query_id, so fixed and rebased as
> of v7 attached.

Overall, this looks good to me.

There are a couple of repetitive comments, like "typmod and collation 
information are irrelevant for the query jumbling".  This applies to all 
nodes, so we don't need to repeat it for a number of nodes (and then not 
mention it for other nodes).  Maybe there should be a central place 
somewhere that describes "these kinds of fields should normally be ignored".