Re: [COMMITTERS] pgsql: Add missing support for new node fields

On Tue, Mar 21, 2017 at 10:56 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
>> On 2017-03-21 07:22:57 +0100, Fabien COELHO wrote:
>>> I've been annoyed by these stupid functions and forgetting to update them
>>> since I run into them while trying to fix an issue in pg_stat_statement some
>>> time ago.
>>>
>>> I've started to develop a perl script to generate most of them from headers.
>>> It is not done yet, but it looks that it can work in the end with limited
>>> effort. Currently it works for copy & equal.
>
>> It'd have to do out/read as well imo.
>
> Yeah. A partial solution is pretty much useless. Even with out/read
> support as well, I'm not sure it's not useless, because you'd still
> have to remember to consider places like expression_tree_walker and
> expression_tree_mutator. Those are not really amenable to automatic
> generation AFAICS, because they have to understand the actual semantics
> of each field.

Well, let's not move the goalposts into the outer solar system. There
are plenty of changes to node structure that don't require
expression_tree_walker or expression_tree_mutator to be touched at
all. Expression nodes aren't touched all that frequently compared to
path, plan, etc. nodes.

IMHO, what would be a lot more useful than something that generates
{read,equal,copy,out}funcs.c automatically would be something that
just checks them for trivial errors of omission. For example, if you
read a list of structure members from the appropriate header files and
cross-checked it against the list of structure members mentioned in
the body of a copy/equal/read/outfunc, you'd catch probably 80% of the
mistakes people make right there. If you could also check for a
copy/read/equal/outfunc that ought to have been present but was
omitted entirely, that'd probably up it to 90%.

The idea would be that if you added a field that wasn't supposed to be
copied, you'd have to add something to copyfuncs.c that said, e.g.

/* NOTCOPIED: mymember */

...and the checking script would ignore things so flagged.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company