Re: WIP: a way forward on bootstrap data - Mailing list pgsql-hackers

Here are the results of an evening's desultory hacking on v13.

I was dissatisfied with the fact that we still had several
function-referencing columns that had numeric instead of symbolic
contents, for instance pg_aggregate.aggfnoid.  Of course, the main reason
is that those are declared regproc but reference functions with overloaded
names, which regproc can't handle.  Now that the lookups are being done in
genbki.pl there's no reason why we have to live with that limitation.
In the attached, I've generalized the BKI_LOOKUP(pg_proc) code so that
you can use either regproc-like or regprocedure-like notation, and then
applied that to relevant columns.

I did not like the hard-wired handling of proargtypes and proallargtypes
in genbki.pl; it hardly seems impossible that we'll want similar
conversions for other array-of-OID columns in future.  After a bit of
thought, it seemed like we could allow

    oidvector    proargtypes BKI_LOOKUP(pg_type);

    Oid          proallargtypes[1] BKI_DEFAULT(_null_) BKI_LOOKUP(pg_type);

and just teach genbki.pl that if a lookup rule is attached to
an oidvector or Oid[] column, it means to apply the rule to
each array element individually.

I also changed genbki.pl so that it'd warn about entries that aren't
recognized by the lookup rules.  This seems like a good idea for
catching errors, such as (ahem) applying BKI_LOOKUP to a column
that isn't even an OID.

bootstrap-v13-delta.patch is a diff atop your patch series for the
in-tree files, and convert_oid2name.patch adjusts that script to
make use of the additional conversion capability.

            regards, tom lane

diff --git a/src/backend/catalog/README.data b/src/backend/catalog/README.data
index b7c680c..22ad0f2 100644
*** a/src/backend/catalog/README.data
--- b/src/backend/catalog/README.data
*************** teach Catalog::ParseData() how to expand
*** 62,71 ****
  representation.

  - To aid readability, some values that are references to other catalog
! entries are represented by macros rather than numeric OIDs. This is
! the case for index access methods, opclasses, operators, opfamilies,
! and types. This is done for functions as well, but only if the proname
! is unique.

  Bootstrap Data Conventions
  ==========================
--- 62,103 ----
  representation.

  - To aid readability, some values that are references to other catalog
! entries are represented by names rather than numeric OIDs.  Currently
! this is the case for access methods, functions, operators, opclasses,
! opfamilies, and types.  The rules are as follows:
!
! * Use of names rather than numbers is enabled for a particular catalog
! column by attaching BKI_LOOKUP(lookuprule) to the column's definition,
! where "lookuprule" is pg_am, pg_proc, pg_operator, pg_opclass,
! pg_opfamily, or pg_type.
!
! * In a name-lookup column, all entries must use the name format except
! when writing "0" for InvalidOid.  (If the column is declared regproc,
! you can optionally write "-" instead of "0".)  genbki.pl will warn
! about unrecognized names.
!
! * Access methods are just represented by their names, as are types.
! Type names must match the referenced pg_type entry's typname; you
! do not get to use any aliases such as "integer" for "int4".
!
! * A function can be represented by its proname, if that is unique among
! the pg_proc.dat entries (this works like regproc input).  Otherwise,
! write it as "proname(argtypename,argtypename,...)", like regprocedure.
! The argument type names must be spelled exactly as they are in the
! pg_proc.dat entry's proargtypes field.  Do not insert any spaces.
!
! * Operators are represented by "oprname(lefttype,righttype)", writing the
! type names exactly as they appear in the pg_operator.dat entry's oprleft
! and oprright fields.  (Write 0 for the omitted operand of a unary
! operator.)
!
! * The names of opclasses and opfamilies are only unique within an access
! method, so they are represented by "access_method_name/object_name".
!
! In none of these cases is there any provision for schema-qualification;
! all objects created during bootstrap are expected to be in the pg_catalog
! schema.
!

  Bootstrap Data Conventions
  ==========================
*************** You can also use the duplicate_oids scri
*** 105,111 ****
  build if a duplicate is found.)

  - The OID counter starts at 10000 at bootstrap.  If a catalog row is
! in a table that requires OIDs, but no OID was preassigned by an "OID ="
  clause, then it will receive an OID of 10000 or above.


--- 137,143 ----
  build if a duplicate is found.)

  - The OID counter starts at 10000 at bootstrap.  If a catalog row is
! in a table that requires OIDs, but no OID was preassigned by an "oid =>"
  clause, then it will receive an OID of 10000 or above.


diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 27494d9..f6be50a 100644
*** a/src/backend/catalog/genbki.pl
--- b/src/backend/catalog/genbki.pl
*************** foreach my $row (@{ $catalog_data{pg_opf
*** 169,181 ****
  my %procoids;
  foreach my $row (@{ $catalog_data{pg_proc} })
  {
!     if (defined($procoids{ $row->{proname} }))
      {
!         $procoids{ $row->{proname} } = 'MULTIPLE';
      }
      else
      {
!         $procoids{ $row->{proname} } = $row->{oid};
      }
  }

--- 169,197 ----
  my %procoids;
  foreach my $row (@{ $catalog_data{pg_proc} })
  {
!     # Generate an entry under just the proname (corresponds to regproc lookup)
!     my $prokey = $row->{proname};
!     if (defined($procoids{ $prokey }))
      {
!         $procoids{ $prokey } = 'MULTIPLE';
      }
      else
      {
!         $procoids{ $prokey } = $row->{oid};
!     }
!     # Also generate an entry using proname(proargtypes).  This is not quite
!     # identical to regprocedure lookup because we don't worry much about
!     # special SQL names for types etc; we just use the names in the source
!     # proargtypes field.  These *should* be unique, but do a multiplicity
!     # check anyway.
!     $prokey .= '(' . join(',', split(/\s+/, $row->{proargtypes})) . ')';
!     if (defined($procoids{ $prokey }))
!     {
!         $procoids{ $prokey } = 'MULTIPLE';
!     }
!     else
!     {
!         $procoids{ $prokey } = $row->{oid};
      }
  }

*************** EOM
*** 294,300 ****
          print $def $line;
      }

!     # Open it, unless bootstrap case (create bootstrap does this
      # automatically)
      if (!$catalog->{bootstrap})
      {
--- 310,316 ----
          print $def $line;
      }

!     # Open it, unless it's a bootstrap catalog (create bootstrap does this
      # automatically)
      if (!$catalog->{bootstrap})
      {
*************** EOM
*** 323,375 ****
              $bki_values{$attname} =~ s/\bPGUID\b/$BOOTSTRAP_SUPERUSERID/g;
              $bki_values{$attname} =~ s/\bPGNSP\b/$PG_CATALOG_NAMESPACE/g;

!             # Replace OID macros with OIDs.
!             # If we don't have a unique value to substitute, just do
!             # nothing. This should only happen in the case for functions,
!             # in which case regprocin will complain.
              if ($column->{lookup})
              {
                  my $lookup = $lookup_kind{ $column->{lookup} };
-                 my $lookupoid = $lookup->{ $bki_values{$attname} };
-                 $bki_values{$attname} = $lookupoid
-                   if defined($lookupoid) && $lookupoid ne 'MULTIPLE';
-             }
-         }

!         # Some pg_proc columns contain lists of types, so we must unpack
!         # these and do the lookups on each element in turn.
!         if ($catname eq 'pg_proc')
!         {

!             # proargtypes
!             if ($bki_values{proargtypes})
!             {
!                 my @argtypenames = split /\s+/, $bki_values{proargtypes};
!                 my @argtypeoids;
!                 foreach my $argtypename (@argtypenames)
                  {
!                     my $argtypeoid  = $typeoids{$argtypename};
!                     push @argtypeoids, $argtypeoid;
                  }
!                 $bki_values{proargtypes} = join(' ', @argtypeoids);
!             }
!
!             # proallargtypes
!             if ($bki_values{proallargtypes} ne '_null_')
!             {
!                 $bki_values{proallargtypes} =~ s/[{}]//g;
!                 my @argtypenames = split /,/, $bki_values{proallargtypes};
!                 my @argtypeoids;
!                 foreach my $argtypename (@argtypenames)
                  {
!                     my $argtypeoid  = $typeoids{$argtypename};
!                     push @argtypeoids, $argtypeoid;
                  }
-                 $bki_values{proallargtypes} =
-                     sprintf "{%s}", join(',', @argtypeoids);
              }
          }
!         elsif ($catname eq 'pg_type' and !exists $bki_values{oid_symbol})
          {
              my $symbol = form_pg_type_symbol($bki_values{typname});
              $bki_values{oid_symbol} = $symbol
--- 339,423 ----
              $bki_values{$attname} =~ s/\bPGUID\b/$BOOTSTRAP_SUPERUSERID/g;
              $bki_values{$attname} =~ s/\bPGNSP\b/$PG_CATALOG_NAMESPACE/g;

!             # Replace OID synonyms with OIDs per the appropriate lookup rule.
!             #
!             # If the column type is oidvector or oid[], we have to replace
!             # each element of the array as per the lookup rule.
!             #
!             # If we don't have a unique value to substitute, warn and
!             # leave the entry unchanged.
              if ($column->{lookup})
              {
                  my $lookup = $lookup_kind{ $column->{lookup} };

!                 die "unrecognized BKI_LOOKUP type " . $column->{lookup}
!                   if !defined($lookup);

!                 if ($atttype eq 'oidvector')
                  {
!                     my @lookupnames = split /\s+/, $bki_values{$attname};
!                     my @lookupoids;
!                     foreach my $lookupname (@lookupnames)
!                     {
!                         my $lookupoid = $lookup->{ $lookupname };
!                         if (defined($lookupoid) && $lookupoid ne 'MULTIPLE')
!                         {
!                             $lookupname = $lookupoid;
!                         }
!                         else
!                         {
!                             warn "unresolved OID reference \"$lookupname\" in $catname row " . join(',',
values(%bki_values))
!                                 if $lookupname ne '-' && $lookupname ne '0';
!                         }
!                         push @lookupoids, $lookupname;
!                     }
!                     $bki_values{$attname} = join(' ', @lookupoids);
                  }
!                 elsif ($atttype eq 'oid[]')
                  {
!                     if ($bki_values{$attname} ne '_null_')
!                     {
!                         $bki_values{$attname} =~ s/[{}]//g;
!                         my @lookupnames = split /,/, $bki_values{$attname};
!                         my @lookupoids;
!                         foreach my $lookupname (@lookupnames)
!                         {
!                             my $lookupoid = $lookup->{ $lookupname };
!                             if (defined($lookupoid) && $lookupoid ne 'MULTIPLE')
!                             {
!                                 $lookupname = $lookupoid;
!                             }
!                             else
!                             {
!                                 warn "unresolved OID reference \"$lookupname\" in $catname row " . join(',',
values(%bki_values))
!                                     if $lookupname ne '-' && $lookupname ne '0';
!                             }
!                             push @lookupoids, $lookupname;
!                         }
!                         $bki_values{$attname} =
!                             sprintf "{%s}", join(',', @lookupoids);
!                     }
!                 }
!                 else
!                 {
!                     my $lookupname = $bki_values{$attname};
!                     my $lookupoid = $lookup->{ $lookupname };
!                     if (defined($lookupoid) && $lookupoid ne 'MULTIPLE')
!                     {
!                         $bki_values{$attname} = $lookupoid;
!                     }
!                     else
!                     {
!                         warn "unresolved OID reference \"$lookupname\" in $catname row " . join(',',
values(%bki_values))
!                             if $lookupname ne '-' && $lookupname ne '0';
!                     }
                  }
              }
          }
!
!         # Special hack to generate OID symbols for pg_type entries
!         # that lack one.
!         if ($catname eq 'pg_type' and !exists $bki_values{oid_symbol})
          {
              my $symbol = form_pg_type_symbol($bki_values{typname});
              $bki_values{oid_symbol} = $symbol
diff --git a/src/include/catalog/pg_aggregate.h b/src/include/catalog/pg_aggregate.h
index 226bb07..767bab5 100644
*** a/src/include/catalog/pg_aggregate.h
--- b/src/include/catalog/pg_aggregate.h
***************
*** 31,37 ****
  CATALOG(pg_aggregate,2600) BKI_WITHOUT_OIDS
  {
      /* pg_proc OID of the aggregate itself */
!     regproc        aggfnoid;

      /* aggregate kind, see AGGKIND_ categories below */
      char        aggkind BKI_DEFAULT(n);
--- 31,37 ----
  CATALOG(pg_aggregate,2600) BKI_WITHOUT_OIDS
  {
      /* pg_proc OID of the aggregate itself */
!     regproc        aggfnoid BKI_LOOKUP(pg_proc);

      /* aggregate kind, see AGGKIND_ categories below */
      char        aggkind BKI_DEFAULT(n);
diff --git a/src/include/catalog/pg_amproc.h b/src/include/catalog/pg_amproc.h
index f045bfa..accbe83 100644
*** a/src/include/catalog/pg_amproc.h
--- b/src/include/catalog/pg_amproc.h
*************** CATALOG(pg_amproc,2603)
*** 54,63 ****
      Oid            amprocrighttype BKI_LOOKUP(pg_type);

      /* support procedure index */
!     int16        amprocnum BKI_LOOKUP(pg_type);

      /* OID of the proc */
!     regproc        amproc;
  } FormData_pg_amproc;

  /* ----------------
--- 54,63 ----
      Oid            amprocrighttype BKI_LOOKUP(pg_type);

      /* support procedure index */
!     int16        amprocnum;

      /* OID of the proc */
!     regproc        amproc BKI_LOOKUP(pg_proc);
  } FormData_pg_amproc;

  /* ----------------
diff --git a/src/include/catalog/pg_cast.h b/src/include/catalog/pg_cast.h
index 2674701..f3bc3c0 100644
*** a/src/include/catalog/pg_cast.h
--- b/src/include/catalog/pg_cast.h
*************** CATALOG(pg_cast,2605)
*** 39,45 ****
      Oid            casttarget BKI_LOOKUP(pg_type);

      /* cast function; 0 = binary coercible */
!     Oid            castfunc;

      /* contexts in which cast can be used */
      char        castcontext;
--- 39,45 ----
      Oid            casttarget BKI_LOOKUP(pg_type);

      /* cast function; 0 = binary coercible */
!     Oid            castfunc BKI_LOOKUP(pg_proc);

      /* contexts in which cast can be used */
      char        castcontext;
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 8d5d044..b9d9cfd 100644
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
*************** CATALOG(pg_proc,1255) BKI_BOOTSTRAP BKI_
*** 49,55 ****
      float4        prorows BKI_DEFAULT(0);

      /* element type of variadic array, or 0 */
!     Oid            provariadic BKI_DEFAULT(0);

      /* transforms calls to it during planning */
      regproc        protransform BKI_DEFAULT(0) BKI_LOOKUP(pg_proc);
--- 49,55 ----
      float4        prorows BKI_DEFAULT(0);

      /* element type of variadic array, or 0 */
!     Oid            provariadic BKI_DEFAULT(0) BKI_LOOKUP(pg_type);

      /* transforms calls to it during planning */
      regproc        protransform BKI_DEFAULT(0) BKI_LOOKUP(pg_proc);
*************** CATALOG(pg_proc,1255) BKI_BOOTSTRAP BKI_
*** 90,101 ****
       */

      /* parameter types (excludes OUT params) */
!     oidvector    proargtypes;

  #ifdef CATALOG_VARLEN

      /* all param types (NULL if IN only) */
!     Oid            proallargtypes[1] BKI_DEFAULT(_null_);

      /* parameter modes (NULL if IN only) */
      char        proargmodes[1] BKI_DEFAULT(_null_);
--- 90,101 ----
       */

      /* parameter types (excludes OUT params) */
!     oidvector    proargtypes BKI_LOOKUP(pg_type);

  #ifdef CATALOG_VARLEN

      /* all param types (NULL if IN only) */
!     Oid            proallargtypes[1] BKI_DEFAULT(_null_) BKI_LOOKUP(pg_type);

      /* parameter modes (NULL if IN only) */
      char        proargmodes[1] BKI_DEFAULT(_null_);
diff --git a/src/include/catalog/pg_type.h b/src/include/catalog/pg_type.h
index 381da18..8992fcd 100644
*** a/src/include/catalog/pg_type.h
--- b/src/include/catalog/pg_type.h
*************** CATALOG(pg_type,1247) BKI_BOOTSTRAP BKI_
*** 109,121 ****
       *
       * typelem != 0 and typlen == -1.
       */
!     Oid            typelem BKI_DEFAULT(0);

      /*
       * If there is a "true" array type having this type as element type,
       * typarray links to it.  Zero if no associated "true" array type.
       */
!     Oid            typarray;

      /*
       * I/O conversion procedures for the datatype.
--- 109,121 ----
       *
       * typelem != 0 and typlen == -1.
       */
!     Oid            typelem BKI_DEFAULT(0) BKI_LOOKUP(pg_type);

      /*
       * If there is a "true" array type having this type as element type,
       * typarray links to it.  Zero if no associated "true" array type.
       */
!     Oid            typarray BKI_DEFAULT(0) BKI_LOOKUP(pg_type);

      /*
       * I/O conversion procedures for the datatype.
*** boot-13/convert_oid2name.pl~    Sat Mar 31 07:53:29 2018
--- boot-13/convert_oid2name.pl    Wed Apr  4 21:45:05 2018
***************
*** 129,146 ****
        $row->{oprname}, $typenames{$row->{oprleft}}, $typenames{$row->{oprright}};
  }

! # Proc oid lookup.
  my %procoids;
  foreach my $row (@{ $catalog_data{pg_proc} })
  {
      next if !ref $row;
      if (defined($procoids{ $row->{proname} }))
      {
          $procoids{ $row->{proname} } = 'MULTIPLE';
      }
      else
      {
!         $procoids{ $row->{oid} } = $row->{proname};
      }
  }

--- 129,153 ----
        $row->{oprname}, $typenames{$row->{oprleft}}, $typenames{$row->{oprright}};
  }

! # Proc oid lookup (see lookup_procname).
! my %procshortnames;
! my %proclongnames;
  my %procoids;
  foreach my $row (@{ $catalog_data{pg_proc} })
  {
      next if !ref $row;
+     $procshortnames{ $row->{oid} } = $row->{proname};
+     $proclongnames{ $row->{oid} } = sprintf "%s(%s)",
+       $row->{proname},
+       join(',', map($typenames{$_}, split(/\s+/, $row->{proargtypes})));
+     # We use this to track whether a proname is duplicated.
      if (defined($procoids{ $row->{proname} }))
      {
          $procoids{ $row->{proname} } = 'MULTIPLE';
      }
      else
      {
!         $procoids{ $row->{proname} } = $row->{oid};
      }
  }

***************
*** 186,191 ****
--- 193,200 ----

              if ($catname eq 'pg_proc')
              {
+                 $values{provariadic} = $typenames{$values{provariadic}}
+                   if exists $values{provariadic};
                  $values{prorettype} = $typenames{$values{prorettype}};
                  if ($values{proargtypes})
                  {
***************
*** 211,222 ****
--- 220,236 ----
              }
              elsif ($catname eq 'pg_aggregate')
              {
+                 $values{aggfnoid}     = lookup_procname($values{aggfnoid});
                  $values{aggsortop}     = $opernames{$values{aggsortop}}
                    if exists $values{aggsortop};
                  $values{aggtranstype}  = $typenames{$values{aggtranstype}};
                  $values{aggmtranstype} = $typenames{$values{aggmtranstype}}
                    if exists $values{aggmtranstype};
              }
+             elsif ($catname eq 'pg_am')
+             {
+                 $values{aggfnoid}     = lookup_procname($values{aggfnoid});
+             }
              elsif ($catname eq 'pg_amop')
              {
                  $values{amoplefttype}   = $typenames{$values{amoplefttype}};
***************
*** 232,242 ****
--- 246,258 ----
                  $values{amprocfamily}    = $opfnames{$values{amprocfamily}};
                  $values{amproclefttype}  = $typenames{$values{amproclefttype}};
                  $values{amprocrighttype} = $typenames{$values{amprocrighttype}};
+                 $values{amproc}          = lookup_procname($values{amproc});
              }
              elsif ($catname eq 'pg_cast')
              {
                  $values{castsource} = $typenames{$values{castsource}};
                  $values{casttarget} = $typenames{$values{casttarget}};
+                 $values{castfunc}   = lookup_procname($values{castfunc});
              }
              elsif ($catname eq 'pg_opclass')
              {
***************
*** 255,260 ****
--- 271,277 ----
                    if exists $values{oprcom};
                  $values{oprnegate} = $opernames{$values{oprnegate}}
                    if exists $values{oprnegate};
+                 $values{oprcode}   = lookup_procname($values{oprcode});
              }
              elsif ($catname eq 'pg_opfamily')
              {
***************
*** 266,271 ****
--- 283,295 ----
                  $values{rngsubtype} = $typenames{$values{rngsubtype}};
                  $values{rngsubopc}  = $opcnames{$values{rngsubopc}};
              }
+             elsif ($catname eq 'pg_type')
+             {
+                 $values{typelem}    = $typenames{$values{typelem}}
+                   if exists $values{typelem};
+                 $values{typarray}   = $typenames{$values{typarray}}
+                   if exists $values{typarray};
+             }

              ############################################################

***************
*** 412,417 ****
--- 436,454 ----
      return $hash_str;
  }

+ sub lookup_procname
+ {
+     my $oid = shift;
+     return $oid if !defined($oid) || $oid eq '-' || $oid eq '0';
+     my $shortname = $procshortnames{$oid};
+     return $shortname if defined($shortname) &&
+         defined($procoids{$shortname}) &&
+         $procoids{$shortname} eq $oid;
+     my $longname = $proclongnames{$oid};
+     return $longname if defined($longname);
+     return $oid;
+ }
+
  sub usage
  {
      die <<EOM;