Clarify handling of special-case values in bootstrap catalog data.

I (tgl) originally coded the special case for pg_proc.pronargs as
though it were a kind of default value.  It seems better though to
treat computable columns as an independent concern: this makes the
code clearer, and probably a bit faster too since we needn't do
work inside the per-column loop.

Improve related comments, as well, in the expectation that there
might be more cases like this in future.

John Naylor, some additional comment-hacking by me

Discussion: https://postgr.es/m/CAJVSVGW-D7OobzU=dybVT2JqZAx-4X1yvBJdavBmqQL05Q6CLw@mail.gmail.com

diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 67d17197f9e1faf32b4fc66e7d80277b96545fef..eee7cb3b90e0e433b569b1c34b32be13a9e910cc 100644 (file)
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -292,6 +292,21 @@ sub AddDefaultValues
    my ($row, $schema, $catname) = @_;
    my @missing_fields;
 
+   # Compute special-case column values.
+   # Note: If you add new cases here, you must also teach
+   # strip_default_values() in include/catalog/reformat_dat_file.pl
+   # to delete them.
+   if ($catname eq 'pg_proc')
+   {
+       # pg_proc.pronargs can be derived from proargtypes.
+       if (defined $row->{proargtypes})
+       {
+           my @proargtypes = split /\s+/, $row->{proargtypes};
+           $row->{pronargs} = scalar(@proargtypes);
+       }
+   }
+
+   # Now fill in defaults, and note any columns that remain undefined.
    foreach my $column (@$schema)
    {
        my $attname = $column->{name};
@@ -305,14 +320,6 @@ sub AddDefaultValues
        {
            $row->{$attname} = $column->{default};
        }
-       elsif ($catname eq 'pg_proc'
-           && $attname eq 'pronargs'
-           && defined($row->{proargtypes}))
-       {
-           # pg_proc.pronargs can be derived from proargtypes.
-           my @proargtypes = split /\s+/, $row->{proargtypes};
-           $row->{$attname} = scalar(@proargtypes);
-       }
        else
        {
            # Failed to find a value.
@@ -320,6 +327,7 @@ sub AddDefaultValues
        }
    }
 
+   # Failure to provide all columns is a hard error.
    if (@missing_fields)
    {
        die sprintf "missing values for field(s) %s in %s.dat line %s\n",

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index f643f564a6b8b4ada81987f9da5829dc15a0d70e..66c6c224a8be7a8e8ad78fcb816408b894adf172 100644 (file)
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -30,6 +30,9 @@
 # "convert srctypename to desttypename" for cast functions
 # "less-equal-greater" for B-tree comparison functions
 
+# Note: pronargs is computed when this file is read, so it does not need
+# to be specified in entries here.  See AddDefaultValues() in Catalog.pm.
+
 # Once upon a time these entries were ordered by OID.  Lately it's often
 # been the custom to insert new entries adjacent to related older entries.
 # Try to do one or the other though, don't just insert entries at random.

diff --git a/src/include/catalog/reformat_dat_file.pl b/src/include/catalog/reformat_dat_file.pl
index 8ebbec6c541670ca63c6d134c35a0d51dbf6deff..4d2523c1bf41370201e672168fca69abb510c0e6 100644 (file)
--- a/src/include/catalog/reformat_dat_file.pl
+++ b/src/include/catalog/reformat_dat_file.pl
@@ -177,31 +177,31 @@ foreach my $catname (@catnames)
    close $dat;
 }
 
-# Leave values out if there is a matching default.
+# Remove column values for which there is a matching default,
+# or if the value can be computed from other columns.
 sub strip_default_values
 {
    my ($row, $schema, $catname) = @_;
 
+   # Delete values that match defaults.
    foreach my $column (@$schema)
    {
        my $attname = $column->{name};
        die "strip_default_values: $catname.$attname undefined\n"
          if !defined $row->{$attname};
 
-       # Delete values that match defaults.
        if (defined $column->{default}
            and ($row->{$attname} eq $column->{default}))
        {
            delete $row->{$attname};
        }
+   }
 
-       # Also delete pg_proc.pronargs, since that can be recomputed.
-       if (   $catname eq 'pg_proc'
-           && $attname eq 'pronargs'
-           && defined($row->{proargtypes}))
-       {
-           delete $row->{$attname};
-       }
+   # Delete computed values.  See AddDefaultValues() in Catalog.pm.
+   # Note: This must be done after deleting values matching defaults.
+   if ($catname eq 'pg_proc')
+   {
+       delete $row->{pronargs} if defined $row->{proargtypes};
    }
 }