Feedback-ID: i131946ab:Fastmail
Message-ID: <34491a4e-2664-6ea7-662c-b13d18d2a92d@enterprisedb.com>
Date: Sat, 7 Jan 2023 07:37:49 +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.0
Subject: Re: Generating code for query jumbling through gen_node_support.pl
Content-Language: en-US
To: Michael Paquier <michael@paquier.xyz>,
 Postgres hackers <pgsql-hackers@lists.postgresql.org>
Cc: "Drouvot, Bertrand" <bdrouvot@amazon.com>
References: <Y5BHOUhX3zTH/ig6@paquier.xyz>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <Y5BHOUhX3zTH/ig6@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/34491a4e-2664-6ea7-662c-b13d18d2a92d%40enterprisedb.com>
Precedence: bulk

On 07.12.22 08:56, Michael Paquier wrote:
> The location of the Nodes is quite invasive because we only care about
> that for T_Const now in the query jumbling, and this could be
> compensated with a third pg_node_attr() that tracks for the "int
> location" of a Node whether it should participate in the jumbling or
> not.

The generation script already has a way to identify location fields, by 
($t eq 'int' && $f =~ 'location$'), so you could use that as well.

> There is also an argument where we would want to not include by
> default new fields added to a Node, but that would require added more
> pg_node_attr() than what's done here.

I'm concerned about the large number of additional field annotations 
this adds.  We have been careful so far to document the use of each 
attribute, e.g., *why* does a field not need to be copied etc.  This 
patch adds dozens and dozens of annotations without any explanation at 
all.  Now, the code this replaces also has no documentation, but maybe 
this is the time to add some.