Re: automatically generating node support functions

Just one more thing here ... I really don't like the fact that
gen_node_support.pl's response to unparseable input is to silently
ignore it.  That's maybe tolerable outside a node struct, but
I think we need a higher standard inside.  I experimented with
promoting the commented-out "warn" to "die", and soon learned
that there are two shortcomings:

* We can't cope with the embedded union inside A_Const.
Simplest fix is to move it outside.

* We can't cope with function-pointer fields.  The only real
problem there is that some of them spread across multiple lines,
but really that was a shortcoming we'd have to fix sometime
anyway.

Proposed patch attached.

            regards, tom lane

diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl
index 96af17516a..35af4e231f 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -213,15 +213,34 @@ foreach my $infile (@ARGV)
     }
     $file_content .= $raw_file_content;

-    my $lineno = 0;
+    my $lineno   = 0;
+    my $prevline = '';
     foreach my $line (split /\n/, $file_content)
     {
+        # per-physical-line processing
         $lineno++;
         chomp $line;
         $line =~ s/\s*$//;
         next if $line eq '';
         next if $line =~ /^#(define|ifdef|endif)/;

+        # within a node struct, don't process until we have whole logical line
+        if ($in_struct && $subline > 1)
+        {
+            if ($line =~ m/;$/)
+            {
+                # found the end, re-attach any previous line(s)
+                $line     = $prevline . $line;
+                $prevline = '';
+            }
+            else
+            {
+                # set it aside for a moment
+                $prevline .= $line . ' ';
+                next;
+            }
+        }
+
         # we are analyzing a struct definition
         if ($in_struct)
         {
@@ -394,7 +413,7 @@ foreach my $infile (@ARGV)
             }
             # normal struct field
             elsif ($line =~
-                /^\s*(.+)\s*\b(\w+)(\[\w+\])?\s*(?:pg_node_attr\(([\w(), ]*)\))?;/
+                /^\s*(.+)\s*\b(\w+)(\[[\w\s+]+\])?\s*(?:pg_node_attr\(([\w(), ]*)\))?;/
               )
             {
                 if ($is_node_struct)
@@ -441,13 +460,46 @@ foreach my $infile (@ARGV)
                     $my_field_attrs{$name} = \@attrs;
                 }
             }
-            else
+            # function pointer field
+            elsif ($line =~
+                /^\s*([\w\s*]+)\s*\(\*(\w+)\)\s*\((.*)\)\s*(?:pg_node_attr\(([\w(), ]*)\))?;/
+              )
             {
                 if ($is_node_struct)
                 {
-                    #warn "$infile:$lineno: could not parse \"$line\"\n";
+                    my $type  = $1;
+                    my $name  = $2;
+                    my $args  = $3;
+                    my $attrs = $4;
+
+                    my @attrs;
+                    if ($attrs)
+                    {
+                        @attrs = split /,\s*/, $attrs;
+                        foreach my $attr (@attrs)
+                        {
+                            if (   $attr !~ /^copy_as\(\w+\)$/
+                                && $attr !~ /^read_as\(\w+\)$/
+                                && !elem $attr,
+                                qw(equal_ignore read_write_ignore))
+                            {
+                                die
+                                  "$infile:$lineno: unrecognized attribute \"$attr\"\n";
+                            }
+                        }
+                    }
+
+                    push @my_fields, $name;
+                    $my_field_types{$name} = 'function pointer';
+                    $my_field_attrs{$name} = \@attrs;
                 }
             }
+            else
+            {
+                # We're not too picky about what's outside structs,
+                # but we'd better understand everything inside.
+                die "$infile:$lineno: could not parse \"$line\"\n";
+            }
         }
         # not in a struct
         else
@@ -709,6 +761,12 @@ _equal${n}(const $n *a, const $n *b)
                   unless $equal_ignore;
             }
         }
+        elsif ($t eq 'function pointer')
+        {
+            # we can copy and compare as a scalar
+            print $cff "\tCOPY_SCALAR_FIELD($f);\n"    unless $copy_ignore;
+            print $eff "\tCOMPARE_SCALAR_FIELD($f);\n" unless $equal_ignore;
+        }
         # node type
         elsif ($t =~ /(\w+)\*/ and elem $1, @node_types)
         {
@@ -980,6 +1038,12 @@ _read${n}(void)
                   unless $no_read;
             }
         }
+        elsif ($t eq 'function pointer')
+        {
+            # We don't print these, and we can't read them either
+            die "cannot read function pointer in struct \"$n\" field \"$f\"\n"
+              unless $no_read;
+        }
         # Special treatments of several Path node fields
         elsif ($t eq 'RelOptInfo*' && elem 'write_only_relids', @a)
         {
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index b0c9c5f2ef..98fe1abaa2 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -303,26 +303,26 @@ typedef struct A_Expr

 /*
  * A_Const - a literal constant
+ *
+ * Value nodes are inline for performance.  You can treat 'val' as a node,
+ * as in IsA(&val, Integer).  'val' is not valid if isnull is true.
  */
+union ValUnion
+{
+    Node        node;
+    Integer        ival;
+    Float        fval;
+    Boolean        boolval;
+    String        sval;
+    BitString    bsval;
+};
+
 typedef struct A_Const
 {
     pg_node_attr(custom_copy_equal, custom_read_write, no_read)

     NodeTag        type;
-
-    /*
-     * Value nodes are inline for performance.  You can treat 'val' as a node,
-     * as in IsA(&val, Integer).  'val' is not valid if isnull is true.
-     */
-    union ValUnion
-    {
-        Node        node;
-        Integer        ival;
-        Float        fval;
-        Boolean        boolval;
-        String        sval;
-        BitString    bsval;
-    }            val;
+    union ValUnion val;
     bool        isnull;            /* SQL NULL constant */
     int            location;        /* token location, or -1 if unknown */
 } A_Const;