Re: Making CallContext and InlineCodeBlock less special-case-y - Mailing list pgsql-hackers

I wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>> On 10.07.22 01:50, Tom Lane wrote:
>>> As committed, gen_node_support.pl excludes CallContext and InlineCodeBlock
>>> from getting unneeded support functions via some very ad-hoc code.

>> Couldn't we just enable those support functions?  I think they were just 
>> excluded because they didn't have any before and nobody bothered to make 
>> any.

> Well, we could I suppose, but that path leads to a lot of dead code in
> backend/nodes/ --- obviously these two alone are negligible, but I want
> a story other than "it's a hack" for execnodes.h and the other files
> we exclude from generation of support code.

Here's a proposed patch for this bit.  Again, whether these two
node types have unnecessary support functions is not the point ---
obviously we could afford to waste that much space.  Rather, what
I'm after is to have a more explainable and flexible way of dealing
with the file-level exclusions applied to a lot of other node types.
This patch doesn't make any change in the script's output now, but
it gives us flexibility for the future.

            regards, tom lane

diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl
index 2c06609726..056530a657 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -50,6 +50,8 @@ my @no_equal;
 my @no_read;
 # node types we don't want read/write support for
 my @no_read_write;
+# node types we don't want any support functions for, just node tags
+my @nodetag_only;

 # types that are copied by straight assignment
 my @scalar_types = qw(
@@ -95,7 +97,10 @@ push @scalar_types, qw(EquivalenceClass* EquivalenceMember*);
 # currently not required.
 push @scalar_types, qw(QualCost);

-# Nodes from these input files don't need support functions, just node tags.
+# Nodes from these input files are automatically treated as nodetag_only.
+# In the future we might add explicit pg_node_attr labeling to some of these
+# files and remove them from this list, but for now this is the path of least
+# resistance.
 my @nodetag_only_files = qw(
   nodes/execnodes.h
   access/amapi.h
@@ -113,10 +118,8 @@ my @nodetag_only_files = qw(

 # XXX various things we are not publishing right now to stay level
 # with the manual system
-push @no_copy,  qw(CallContext InlineCodeBlock);
-push @no_equal, qw(CallContext InlineCodeBlock);
 push @no_read_write,
-  qw(AccessPriv AlterTableCmd CallContext CreateOpClassItem FunctionParameter InferClause InlineCodeBlock
ObjectWithArgsOnConflictClause PartitionCmd RoleSpec VacuumRelation); 
+  qw(AccessPriv AlterTableCmd CreateOpClassItem FunctionParameter InferClause ObjectWithArgs OnConflictClause
PartitionCmdRoleSpec VacuumRelation); 
 push @no_read, qw(A_ArrayExpr A_Indices A_Indirection AlterStatsStmt
   CollateClause ColumnDef ColumnRef CreateForeignTableStmt CreateStatsStmt
   CreateStmt FuncCall ImportForeignSchemaStmt IndexElem IndexStmt
@@ -254,6 +257,10 @@ foreach my $infile (@ARGV)
                         {
                             push @no_read, $in_struct;
                         }
+                        elsif ($attr eq 'nodetag_only')
+                        {
+                            push @nodetag_only, $in_struct;
+                        }
                         elsif ($attr eq 'special_read_write')
                         {
                             # This attribute is called
@@ -314,13 +321,9 @@ foreach my $infile (@ARGV)
                     $node_type_info{$in_struct}->{field_types} = \%ft;
                     $node_type_info{$in_struct}->{field_attrs} = \%fa;

-                    # Exclude nodes in nodetag_only_files from support.
-                    if (elem $infile, @nodetag_only_files)
-                    {
-                        push @no_copy,       $in_struct;
-                        push @no_equal,      $in_struct;
-                        push @no_read_write, $in_struct;
-                    }
+                    # Propagate nodetag_only marking from files to nodes
+                    push @nodetag_only, $in_struct
+                      if (elem $infile, @nodetag_only_files);

                     # Propagate some node attributes from supertypes
                     if ($supertype)
@@ -515,6 +518,7 @@ print $eff $node_includes;
 foreach my $n (@node_types)
 {
     next if elem $n, @abstract_types;
+    next if elem $n, @nodetag_only;
     my $struct_no_copy  = (elem $n, @no_copy);
     my $struct_no_equal = (elem $n, @no_equal);
     next if $struct_no_copy && $struct_no_equal;
@@ -706,6 +710,7 @@ print $rff $node_includes;
 foreach my $n (@node_types)
 {
     next if elem $n, @abstract_types;
+    next if elem $n, @nodetag_only;
     next if elem $n, @no_read_write;

     # XXX For now, skip all "Stmt"s except that ones that were there before.
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index adc549002a..c8ed4e0552 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -61,12 +61,17 @@ typedef enum NodeTag
  *
  * - no_read: Does not support nodeRead() at all.
  *
+ * - nodetag_only: Does not support copyObject(), equal(), outNode(),
+ *   or nodeRead().
+ *
  * - special_read_write: Has special treatment in outNode() and nodeRead().
  *
  * Node types can be supertypes of other types whether or not they are marked
  * abstract: if a node struct appears as the first field of another struct
  * type, then it is the supertype of that type.  The no_copy, no_equal, and
  * no_read node attributes are automatically inherited from the supertype.
+ * (Notice that nodetag_only does not inherit, so it's not quite equivalent
+ * to a combination of other attributes.)
  *
  * Valid node field attributes:
  *
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 0b6a7bb365..e2ad761768 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3382,6 +3382,8 @@ typedef struct DoStmt

 typedef struct InlineCodeBlock
 {
+    pg_node_attr(nodetag_only)    /* this is not a member of parse trees */
+
     NodeTag        type;
     char       *source_text;    /* source text of anonymous code block */
     Oid            langOid;        /* OID of selected language */
@@ -3408,6 +3410,8 @@ typedef struct CallStmt

 typedef struct CallContext
 {
+    pg_node_attr(nodetag_only)    /* this is not a member of parse trees */
+
     NodeTag        type;
     bool        atomic;
 } CallContext;