Re: Removing another gen_node_support.pl special case - Mailing list pgsql-hackers

I wrote:
> I notice that EquivalenceClass is already marked as no_copy_equal,
> which means that gen_node_support.pl can know that emitting a
> recursive node-copy or node-compare request is a bad idea.  What
> do you think of using the patch as it stands, plus a cross-check
> that we don't emit COPY_NODE_FIELD or COMPARE_NODE_FIELD if the
> target node type is no_copy or no_equal?

Concretely, it seems like something like the attached could be
useful, independently of the other change.

            regards, tom lane

diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl
index b6f086e262..fc5b6721d6 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -123,6 +123,8 @@ my @no_equal;
 my @no_read;
 # node types we don't want read/write support for
 my @no_read_write;
+# node types that have handmade read/write support
+my @special_read_write;
 # node types we don't want any support functions for, just node tags
 my @nodetag_only;

@@ -152,9 +154,12 @@ my @extra_tags = qw(
 # since we won't use its internal structure here anyway.
 push @node_types, qw(List);
 # Lists are specially treated in all four support files, too.
-push @no_copy,       qw(List);
-push @no_equal,      qw(List);
-push @no_read_write, qw(List);
+# (Ideally we'd mark List as "special copy/equal" not "no copy/equal".
+# But until there's other use-cases for that, just hot-wire the tests
+# that would need to distinguish.)
+push @no_copy,            qw(List);
+push @no_equal,           qw(List);
+push @special_read_write, qw(List);

 # Nodes with custom copy/equal implementations are skipped from
 # .funcs.c but need case statements in .switch.c.
@@ -338,16 +343,7 @@ foreach my $infile (@ARGV)
                         }
                         elsif ($attr eq 'special_read_write')
                         {
-                            # This attribute is called
-                            # "special_read_write" because there is
-                            # special treatment in outNode() and
-                            # nodeRead() for these nodes.  For this
-                            # script, it's the same as
-                            # "no_read_write", but calling the
-                            # attribute that externally would probably
-                            # be confusing, since read/write support
-                            # does in fact exist.
-                            push @no_read_write, $in_struct;
+                            push @special_read_write, $in_struct;
                         }
                         elsif ($attr =~ /^nodetag_number\((\d+)\)$/)
                         {
@@ -786,6 +782,17 @@ _equal${n}(const $n *a, const $n *b)
         elsif (($t =~ /^(\w+)\*$/ or $t =~ /^struct\s+(\w+)\*$/)
             and elem $1, @node_types)
         {
+            die
+              "node type \"$1\" lacks copy support, which is required for struct \"$n\" field \"$f\"\n"
+              if (elem $1, @no_copy or elem $1, @nodetag_only)
+              and $1 ne 'List'
+              and !$copy_ignore;
+            die
+              "node type \"$1\" lacks equal support, which is required for struct \"$n\" field \"$f\"\n"
+              if (elem $1, @no_equal or elem $1, @nodetag_only)
+              and $1 ne 'List'
+              and !$equal_ignore;
+
             print $cff "\tCOPY_NODE_FIELD($f);\n"    unless $copy_ignore;
             print $eff "\tCOMPARE_NODE_FIELD($f);\n" unless $equal_ignore;
         }
@@ -851,6 +858,7 @@ foreach my $n (@node_types)
     next if elem $n, @abstract_types;
     next if elem $n, @nodetag_only;
     next if elem $n, @no_read_write;
+    next if elem $n, @special_read_write;

     my $no_read = (elem $n, @no_read);

@@ -1083,6 +1091,14 @@ _read${n}(void)
         elsif (($t =~ /^(\w+)\*$/ or $t =~ /^struct\s+(\w+)\*$/)
             and elem $1, @node_types)
         {
+            die
+              "node type \"$1\" lacks write support, which is required for struct \"$n\" field \"$f\"\n"
+              if (elem $1, @no_read_write or elem $1, @nodetag_only);
+            die
+              "node type \"$1\" lacks read support, which is required for struct \"$n\" field \"$f\"\n"
+              if (elem $1, @no_read or elem $1, @nodetag_only)
+              and !$no_read;
+
             print $off "\tWRITE_NODE_FIELD($f);\n";
             print $rff "\tREAD_NODE_FIELD($f);\n" unless $no_read;
         }