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

From Tom Lane
Subject Re: WIP: a way forward on bootstrap data
Date
Msg-id 916.1523046045@sss.pgh.pa.us
Whole thread Raw
In response to Re: WIP: a way forward on bootstrap data  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: WIP: a way forward on bootstrap data
List pgsql-hackers
Oh, one more thing: looking again at the contents of pg_proc.dat,
I find myself annoyed at the need to specify pronargs.  That's
entirely derivable from proargtypes, and if we did so, we'd get
down to this for the first few pg_proc entries:

{ oid => '1242', descr => 'I/O',
  proname => 'boolin', prorettype => 'bool', proargtypes => 'cstring',
  prosrc => 'boolin' },
{ oid => '1243', descr => 'I/O',
  proname => 'boolout', prorettype => 'cstring', proargtypes => 'bool',
  prosrc => 'boolout' },
{ oid => '1244', descr => 'I/O',
  proname => 'byteain', prorettype => 'bytea', proargtypes => 'cstring',
  prosrc => 'byteain' },

which seems pretty close to the minimum amount of stuff you could expect
to need to write.

I recall that this and some other data compression methods were on the
table awhile back, and I expressed doubt about putting very much magic
knowledge in genbki.pl, but this case seems like a no-brainer.  genbki
can always get the right answer, and expecting people to do it just
creates another way to make a mistake.

So attached is an incremental patch to do that.  I had to adjust
AddDefaultValues's argument list to tell it the catalog name,
and once I did that, I saw that the API arrangement whereby the
caller throws error was just useless complexity, so I got rid of it.

            regards, tom lane

diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 70aea89..3b3bb6b 100644
*** a/src/backend/catalog/Catalog.pm
--- b/src/backend/catalog/Catalog.pm
*************** sub ParseData
*** 254,265 ****
                  }

                  # Expand tuples to their full representation.
!                 my $error = AddDefaultValues($hash_ref, $schema);
!                 if ($error)
!                 {
!                     print "Failed to form full tuple for $catname\n";
!                     die $error;
!                 }
              }
              else
              {
--- 254,260 ----
                  }

                  # Expand tuples to their full representation.
!                 AddDefaultValues($hash_ref, $schema, $catname);
              }
              else
              {
*************** sub ParseData
*** 289,302 ****
      return $data;
  }

! # Fill in default values of a record using the given schema. It's the
! # caller's responsibility to specify other values beforehand.
! # If we fail to fill all fields, return a nonempty error message.
  sub AddDefaultValues
  {
!     my ($row, $schema) = @_;
      my @missing_fields;
-     my $msg;

      foreach my $column (@$schema)
      {
--- 284,295 ----
      return $data;
  }

! # Fill in default values of a record using the given schema.
! # It's the caller's responsibility to specify other values beforehand.
  sub AddDefaultValues
  {
!     my ($row, $schema, $catname) = @_;
      my @missing_fields;

      foreach my $column (@$schema)
      {
*************** sub AddDefaultValues
*** 311,316 ****
--- 304,316 ----
          {
              $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.
*************** sub AddDefaultValues
*** 320,333 ****

      if (@missing_fields)
      {
!         $msg = "Missing values for: " . join(', ', @missing_fields);
!         $msg .= "\nShowing other values for context:\n";
          while (my($key, $value) = each %$row)
          {
              $msg .= "$key => $value, ";
          }
      }
-     return $msg;
  }

  # Rename temporary files to final names.
--- 320,334 ----

      if (@missing_fields)
      {
!         my $msg = "Failed to form full tuple for $catname\n";
!         $msg .= "Missing values for: " . join(', ', @missing_fields);
!         $msg .= "\nOther values for row:\n";
          while (my($key, $value) = each %$row)
          {
              $msg .= "$key => $value, ";
          }
+         die $msg;
      }
  }

  # Rename temporary files to final names.
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index d4c2ddf..3512952 100644
*** a/src/backend/catalog/genbki.pl
--- b/src/backend/catalog/genbki.pl
*************** sub morph_row_for_pgattr
*** 641,651 ****
          $row->{attnotnull} = 'f';
      }

!     my $error = Catalog::AddDefaultValues($row, $pgattr_schema);
!     if ($error)
!     {
!         die "Failed to form full tuple for pg_attribute: ", $error;
!     }
  }

  # Write an entry to postgres.bki. Adding quotes here allows us to keep
--- 641,647 ----
          $row->{attnotnull} = 'f';
      }

!     Catalog::AddDefaultValues($row, $pgattr_schema, 'pg_attribute');
  }

  # Write an entry to postgres.bki. Adding quotes here allows us to keep
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 2c42b5c..b25546e 100644
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
*************** CATALOG(pg_proc,1255,ProcedureRelationId
*** 73,78 ****
--- 73,79 ----
      char        proparallel BKI_DEFAULT(s);

      /* number of arguments */
+     /* Note: need not be given in pg_proc.dat; genbki.pl will compute it */
      int16        pronargs;

      /* number of arguments with defaults */
diff --git a/src/include/catalog/reformat_dat_file.pl b/src/include/catalog/reformat_dat_file.pl
index b20d236..bbceb16 100644
*** a/src/include/catalog/reformat_dat_file.pl
--- b/src/include/catalog/reformat_dat_file.pl
*************** sub strip_default_values
*** 201,206 ****
--- 201,213 ----
          {
              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};
+         }
      }
  }


pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Documentation for bootstrap data conversion
Next
From: Claudio Freire
Date:
Subject: Re: Vacuum: allow usage of more than 1GB of work mem