I wrote:
> Andres Freund <[email protected]> writes:
>> Additionally, I think we've had to add tags to the enum in minor releases
>> before and I'm afraid this now would end up looking even more awkward?
> Peter and I already had a discussion about that upthread --- we figured
> that if there's a way to manually assign a nodetag's number, you could use
> that option when you have to add a tag in a stable branch. We didn't
> actually build out that idea, but I can go do that, if we can solve the
> more fundamental problem of keeping the autogenerated numbers stable.
> One issue with that idea, of course, is that you have to remember to do
> it like that when back-patching a node addition. Ideally there'd be
> something that'd carp if the last autogenerated tag moves in a stable
> branch, but I'm not very sure where to put that.
One way to do it is to provide logic in gen_node_support.pl to check
that, and activate that logic only in back branches. If we make that
part of the branch-making procedure, we'd not forget to do it.
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 2c06609726..2c6766f537 100644
--- a/src/backend/nodes/gen_node_support.pl
+++ b/src/backend/nodes/gen_node_support.pl
@@ -34,6 +34,20 @@ sub elem
return grep { $_ eq $x } @_;
}
+
+# ARM ABI STABILITY CHECK HERE:
+#
+# In stable branches, set $last_nodetag to the name of the last node type
+# that should receive an auto-generated nodetag number, and $last_nodetag_no
+# to its number. The script will then complain if those values don't match
+# reality, providing a cross-check that we haven't broken ABI by adding or
+# removing nodetags.
+# In HEAD, these variables should be left undef, since we don't promise
+# ABI stability during development.
+
+my $last_nodetag = undef;
+my $last_nodetag_no = undef;
+
# output file names
my @output_files;
@@ -88,6 +102,9 @@ my @custom_copy_equal;
# Similarly for custom read/write implementations.
my @custom_read_write;
+# Track node types with manually assigned NodeTag numbers.
+my %manual_nodetag_number;
+
# EquivalenceClasses are never moved, so just shallow-copy the pointer
push @scalar_types, qw(EquivalenceClass* EquivalenceMember*);
@@ -267,6 +284,10 @@ foreach my $infile (@ARGV)
# does in fact exist.
push @no_read_write, $in_struct;
}
+ elsif ($attr =~ /^nodetag_number\((\d+)\)$/)
+ {
+ $manual_nodetag_number{$in_struct} = $1;
+ }
else
{
die
@@ -472,14 +493,31 @@ open my $nt, '>', 'nodetags.h' . $tmpext or die $!;
printf $nt $header_comment, 'nodetags.h';
-my $i = 1;
+my $tagno = 0;
+my $last_tag = undef;
foreach my $n (@node_types, @extra_tags)
{
next if elem $n, @abstract_types;
- print $nt "\tT_${n} = $i,\n";
- $i++;
+ if (defined $manual_nodetag_number{$n})
+ {
+ # do not change $tagno or $last_tag
+ print $nt "\tT_${n} = $manual_nodetag_number{$n},\n";
+ }
+ else
+ {
+ $tagno++;
+ $last_tag = $n;
+ print $nt "\tT_${n} = $tagno,\n";
+ }
}
+# verify that last auto-assigned nodetag stays stable
+die "ABI stability break: last nodetag is $last_tag not $last_nodetag\n"
+ if (defined $last_nodetag && $last_nodetag ne $last_tag);
+die
+ "ABI stability break: last nodetag number is $tagno not $last_nodetag_no\n"
+ if (defined $last_nodetag_no && $last_nodetag_no ne $tagno);
+
close $nt;
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index adc549002a..e0b336cd28 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -63,6 +63,11 @@ typedef enum NodeTag
*
* - special_read_write: Has special treatment in outNode() and nodeRead().
*
+ * - nodetag_number(VALUE): assign the specified nodetag number instead of
+ * an auto-generated number. Typically this would only be used in stable
+ * branches, to give a newly-added node type a number without breaking ABI
+ * by changing the numbers of existing node types.
+ *
* 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
diff --git a/src/tools/RELEASE_CHANGES b/src/tools/RELEASE_CHANGES
index e8de724fcd..73b02fa2a4 100644
--- a/src/tools/RELEASE_CHANGES
+++ b/src/tools/RELEASE_CHANGES
@@ -107,6 +107,10 @@ Starting a New Development Cycle
placeholder), "git rm" the previous one, and update release.sgml and
filelist.sgml to match.
+* In the newly-made branch, change src/backend/nodes/gen_node_support.pl
+ to enforce ABI stability of the NodeTag list (see "ARM ABI STABILITY
+ CHECK HERE" therein).
+
* Notify the private committers email list, to ensure all committers
are aware of the new branch even if they're not paying close attention
to pgsql-hackers.
