Re: automatically generating node support functions

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> Here is a patch that reformats the relevant (and a few more) comments 
> that way.  This has been run through pgindent, so the formatting should 
> be stable.

Now that that's been pushed, the main patch is of course quite broken.
Are you working on a rebase?

I looked through the last published version of the main patch (Alvaro's
0002 from 2022-04-19), without trying to actually test it, and found
a couple of things that look wrong in the Makefiles:

* AFAICT, the infrastructure for removing the generated files at
"make *clean" is incomplete.  In particular I don't see any code
for removing the symlinks or the associated stamp file during
"make clean".  It looks like the existing header symlinks are
all cleaned up in src/include/Makefile's "clean" rule, so you
could do likewise for these.  Also, the "make maintainer-clean"
infrastructure seems incomplete --- shouldn't src/backend/Makefile's
maintainer-clean rule now also do
    $(MAKE) -C nodes $@
?

* There are some useful comments in backend/utils/Makefile that
I think should be carried over along with the make rules that
(it looks like) you cribbed from there, notably

# fmgr-stamp records the last time we ran Gen_fmgrtab.pl.  We don't rely on
# the timestamps of the individual output files, because the Perl script
# won't update them if they didn't change (to avoid unnecessary recompiles).

# These generated headers must be symlinked into builddir/src/include/,
# using absolute links for the reasons explained in src/backend/Makefile.
# We use header-stamp to record that we've done this because the symlinks
# themselves may appear older than fmgr-stamp.

and something similar to this for the "clean" rule:
# fmgroids.h, fmgrprotos.h, fmgrtab.c, fmgr-stamp, and errcodes.h are in the
# distribution tarball, so they are not cleaned here.


Also, I share David's upthread allergy to the option names "path_hackN"
and to documenting those only inside the conversion script.  I think
the existing text that you moved into the script, such as this bit:

        # We do not print the parent, else we'd be in infinite
        # recursion.  We can print the parent's relids for
        # identification purposes, though.  We print the pathtarget
        # only if it's not the default one for the rel.  We also do
        # not print the whole of param_info, since it's printed via
        # RelOptInfo; it's sufficient and less cluttering to print
        # just the required outer relids.

is perfectly adequate as documentation, it just needs to be somewhere else
(pathnodes.h seems fine, if not nodes.h) and labeled as to exactly which
pg_node_attr option invokes which behavior.

BTW, I think this: "Unknown attributes are ignored" is a seriously
bad idea; it will allow typos to escape detection.

            regards, tom lane