Remove hard-coded schema knowledge about pg_attribute from genbki.pl
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Fri, 12 Jan 2018 14:21:42 +0000 (11:21 -0300)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Fri, 12 Jan 2018 14:21:42 +0000 (11:21 -0300)
Add the ability to label a column's default value in the catalog header,
and implement this for pg_attribute.  A new function in Catalog.pm is
used to fill in a tuple with defaults.  The build process will complain
loudly if a catalog entry is incomplete,

Commit 8137f2c3232 labeled variable length columns for the C preprocessor.
Expose that label to genbki.pl so we can exclude those columns from schema
macros in a general fashion. Also, format schema macro entries according
to their types.

This means slightly less code maintenance, but more importantly it's a
proving ground for mechanisms intended to be used in later commits.

While at it, I (Álvaro) couldn't resist making some changes in
genbki.pl: rename some functions to actually indicate their purpose
instead of actively misleading onlookers; and don't iterate on the whole
of pg_type to find the entry for each catalog row, using a hash instead
of an array.

Author: John Naylor, some changes by Álvaro Herrera
Discussion: https://postgr.es/m/CAJVSVGVJHwD8sfDfZW9TbCHWKf=C1YDRM-rF=2JenRU_y+VcFg@mail.gmail.com

src/backend/catalog/Catalog.pm
src/backend/catalog/genbki.pl
src/include/catalog/genbki.h
src/include/catalog/pg_attribute.h

index f18b400bd71d21fdc50c87190fe67294acffb020..9ced1547f6bbe4801c0abf24ebc4e2a5bf5b7268 100644 (file)
@@ -37,6 +37,8 @@ sub Catalogs
    foreach my $input_file (@_)
    {
        my %catalog;
+       my $is_varlen     = 0;
+
        $catalog{columns} = [];
        $catalog{data}    = [];
 
@@ -164,7 +166,11 @@ sub Catalogs
            elsif ($declaring_attributes)
            {
                next if (/^{|^$/);
-               next if (/^#/);
+               if (/^#/)
+               {
+                   $is_varlen = 1 if /^#ifdef\s+CATALOG_VARLEN/;
+                   next;
+               }
                if (/^}/)
                {
                    undef $declaring_attributes;
@@ -172,8 +178,12 @@ sub Catalogs
                else
                {
                    my %column;
-                   my ($atttype, $attname, $attopt) = split /\s+/, $_;
-                   die "parse error ($input_file)" unless $attname;
+                   my @attopts = split /\s+/, $_;
+                   my $atttype = shift @attopts;
+                   my $attname = shift @attopts;
+                   die "parse error ($input_file)"
+                     unless ($attname and $atttype);
+
                    if (exists $RENAME_ATTTYPE{$atttype})
                    {
                        $atttype = $RENAME_ATTTYPE{$atttype};
@@ -181,13 +191,14 @@ sub Catalogs
                    if ($attname =~ /(.*)\[.*\]/)    # array attribute
                    {
                        $attname = $1;
-                       $atttype .= '[]';            # variable-length only
+                       $atttype .= '[]';
                    }
 
                    $column{type} = $atttype;
                    $column{name} = $attname;
+                   $column{is_varlen} = 1 if $is_varlen;
 
-                   if (defined $attopt)
+                   foreach my $attopt (@attopts)
                    {
                        if ($attopt eq 'BKI_FORCE_NULL')
                        {
@@ -197,11 +208,20 @@ sub Catalogs
                        {
                            $column{forcenotnull} = 1;
                        }
+                       elsif ($attopt =~ /BKI_DEFAULT\((\S+)\)/)
+                       {
+                           $column{default} = $1;
+                       }
                        else
                        {
                            die
 "unknown column option $attopt on column $attname";
                        }
+
+                       if ($column{forcenull} and $column{forcenotnull})
+                       {
+                           die "$attname is forced both null and not null";
+                       }
                    }
                    push @{ $catalog{columns} }, \%column;
                }
@@ -235,6 +255,46 @@ sub SplitDataLine
    return @result;
 }
 
+# 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) = @_;
+   my @missing_fields;
+   my $msg;
+
+   foreach my $column (@$schema)
+   {
+       my $attname = $column->{name};
+       my $atttype = $column->{type};
+
+       if (defined $row->{$attname})
+       {
+           ;
+       }
+       elsif (defined $column->{default})
+       {
+           $row->{$attname} = $column->{default};
+       }
+       else
+       {
+           # Failed to find a value.
+           push @missing_fields, $attname;
+       }
+   }
+
+   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.
 # Call this function with the final file name and the .tmp extension
 # Note: recommended extension is ".tmp$$", so that parallel make steps
index 1d3bbcc532d47cb0c2b97a30dc152eb2c3fb4132..ed90a0230357688ac734b8d2be08c178959a900c 100644 (file)
@@ -105,7 +105,7 @@ print $bki "# PostgreSQL $major_version\n";
 my %schemapg_entries;
 my @tables_needing_macros;
 my %regprocoids;
-my @types;
+my %types;
 
 # produce output, one catalog at a time
 foreach my $catname (@{ $catalogs->{names} })
@@ -119,7 +119,6 @@ foreach my $catname (@{ $catalogs->{names} })
      . $catalog->{without_oids}
      . $catalog->{rowtype_oid} . "\n";
 
-   my %bki_attr;
    my @attnames;
    my $first = 1;
 
@@ -129,7 +128,6 @@ foreach my $catname (@{ $catalogs->{names} })
    {
        my $attname = $column->{name};
        my $atttype = $column->{type};
-       $bki_attr{$attname} = $column;
        push @attnames, $attname;
 
        if (!$first)
@@ -211,7 +209,7 @@ foreach my $catname (@{ $catalogs->{names} })
            {
                my %type = %bki_values;
                $type{oid} = $row->{oid};
-               push @types, \%type;
+               $types{ $type{typname} } = \%type;
            }
 
            # Write to postgres.bki
@@ -253,28 +251,24 @@ foreach my $catname (@{ $catalogs->{names} })
            # Generate entries for user attributes.
            my $attnum       = 0;
            my $priornotnull = 1;
-           my @user_attrs   = @{ $table->{columns} };
-           foreach my $attr (@user_attrs)
+           foreach my $attr (@{ $table->{columns} })
            {
                $attnum++;
-               my $row = emit_pgattr_row($table_name, $attr, $priornotnull);
-               $row->{attnum}        = $attnum;
-               $row->{attstattarget} = '-1';
-               $priornotnull &= ($row->{attnotnull} eq 't');
+               my %row;
+               $row{attnum}   = $attnum;
+               $row{attrelid} = $table->{relation_oid};
+
+               morph_row_for_pgattr(\%row, $schema, $attr, $priornotnull);
+               $priornotnull &= ($row{attnotnull} eq 't');
 
                # If it's bootstrapped, put an entry in postgres.bki.
-               if ($table->{bootstrap})
-               {
-                   bki_insert($row, @attnames);
-               }
+               print_bki_insert(\%row, @attnames) if $table->{bootstrap};
 
                # Store schemapg entries for later.
-               $row =
-                 emit_schemapg_row($row,
-                   grep { $bki_attr{$_}{type} eq 'bool' } @attnames);
+               morph_row_for_schemapg(\%row, $schema);
                push @{ $schemapg_entries{$table_name} },
                  sprintf "{ %s }",
-                   join(', ', grep { defined $_ } @{$row}{@attnames});
+                   join(', ', grep { defined $_ } @row{@attnames});
            }
 
            # Generate entries for system attributes.
@@ -293,16 +287,18 @@ foreach my $catname (@{ $catalogs->{names} })
                foreach my $attr (@SYS_ATTRS)
                {
                    $attnum--;
-                   my $row = emit_pgattr_row($table_name, $attr, 1);
-                   $row->{attnum}        = $attnum;
-                   $row->{attstattarget} = '0';
+                   my %row;
+                   $row{attnum}        = $attnum;
+                   $row{attrelid}      = $table->{relation_oid};
+                   $row{attstattarget} = '0';
 
                    # Omit the oid column if the catalog doesn't have them
                    next
                      if $table->{without_oids}
-                         && $row->{attname} eq 'oid';
+                         && $attr->{name} eq 'oid';
 
-                   bki_insert($row, @attnames);
+                   morph_row_for_pgattr(\%row, $schema, $attr, 1);
+                   print_bki_insert(\%row, @attnames);
                }
            }
        }
@@ -379,130 +375,122 @@ exit 0;
 #################### Subroutines ########################
 
 
-# Given a system catalog name and a reference to a key-value pair corresponding
-# to the name and type of a column, generate a reference to a hash that
-# represents a pg_attribute entry.  We must also be told whether preceding
-# columns were all not-null.
-sub emit_pgattr_row
+# Given $pgattr_schema (the pg_attribute schema for a catalog sufficient for
+# AddDefaultValues), $attr (the description of a catalog row), and
+# $priornotnull (whether all prior attributes in this catalog are not null),
+# modify the $row hashref for print_bki_insert.  This includes setting data
+# from the corresponding pg_type element and filling in any default values.
+# Any value not handled here must be supplied by caller.
+sub morph_row_for_pgattr
 {
-   my ($table_name, $attr, $priornotnull) = @_;
+   my ($row, $pgattr_schema, $attr, $priornotnull) = @_;
    my $attname = $attr->{name};
    my $atttype = $attr->{type};
-   my %row;
 
-   $row{attrelid} = $catalogs->{$table_name}->{relation_oid};
-   $row{attname}  = $attname;
+   $row->{attname} = $attname;
 
-   # Adjust type name for arrays: foo[] becomes _foo
-   # so we can look it up in pg_type
-   if ($atttype =~ /(.+)\[\]$/)
-   {
-       $atttype = '_' . $1;
-   }
+   # Adjust type name for arrays: foo[] becomes _foo, so we can look it up in
+   # pg_type
+   $atttype = '_' . $1 if $atttype =~ /(.+)\[\]$/;
 
    # Copy the type data from pg_type, and add some type-dependent items
-   foreach my $type (@types)
-   {
-       if (defined $type->{typname} && $type->{typname} eq $atttype)
-       {
-           $row{atttypid}   = $type->{oid};
-           $row{attlen}     = $type->{typlen};
-           $row{attbyval}   = $type->{typbyval};
-           $row{attstorage} = $type->{typstorage};
-           $row{attalign}   = $type->{typalign};
+   my $type = $types{$atttype};
 
-           # set attndims if it's an array type
-           $row{attndims} = $type->{typcategory} eq 'A' ? '1' : '0';
-           $row{attcollation} = $type->{typcollation};
+   $row->{atttypid}   = $type->{oid};
+   $row->{attlen}     = $type->{typlen};
+   $row->{attbyval}   = $type->{typbyval};
+   $row->{attstorage} = $type->{typstorage};
+   $row->{attalign}   = $type->{typalign};
 
-           if (defined $attr->{forcenotnull})
-           {
-               $row{attnotnull} = 't';
-           }
-           elsif (defined $attr->{forcenull})
-           {
-               $row{attnotnull} = 'f';
-           }
-           elsif ($priornotnull)
-           {
+   # set attndims if it's an array type
+   $row->{attndims} = $type->{typcategory} eq 'A' ? '1' : '0';
+   $row->{attcollation} = $type->{typcollation};
 
-               # attnotnull will automatically be set if the type is
-               # fixed-width and prior columns are all NOT NULL ---
-               # compare DefineAttr in bootstrap.c. oidvector and
-               # int2vector are also treated as not-nullable.
-               $row{attnotnull} =
-                   $type->{typname} eq 'oidvector'   ? 't'
-                 : $type->{typname} eq 'int2vector'  ? 't'
-                 : $type->{typlen}  eq 'NAMEDATALEN' ? 't'
-                 : $type->{typlen} > 0 ? 't'
-                 :                       'f';
-           }
-           else
-           {
-               $row{attnotnull} = 'f';
-           }
-           last;
-       }
+   if (defined $attr->{forcenotnull})
+   {
+       $row->{attnotnull} = 't';
+   }
+   elsif (defined $attr->{forcenull})
+   {
+       $row->{attnotnull} = 'f';
    }
+   elsif ($priornotnull)
+   {
 
-   # Add in default values for pg_attribute
-   my %PGATTR_DEFAULTS = (
-       attcacheoff   => '-1',
-       atttypmod     => '-1',
-       atthasdef     => 'f',
-       attidentity   => '',
-       attisdropped  => 'f',
-       attislocal    => 't',
-       attinhcount   => '0',
-       attacl        => '_null_',
-       attoptions    => '_null_',
-       attfdwoptions => '_null_');
-   return { %PGATTR_DEFAULTS, %row };
+       # attnotnull will automatically be set if the type is
+       # fixed-width and prior columns are all NOT NULL ---
+       # compare DefineAttr in bootstrap.c. oidvector and
+       # int2vector are also treated as not-nullable.
+       $row->{attnotnull} =
+       $type->{typname} eq 'oidvector'   ? 't'
+       : $type->{typname} eq 'int2vector'  ? 't'
+       : $type->{typlen}  eq 'NAMEDATALEN' ? 't'
+       : $type->{typlen} > 0 ? 't'
+       :                       'f';
+   }
+   else
+   {
+       $row->{attnotnull} = 'f';
+   }
+
+   my $error = Catalog::AddDefaultValues($row, $pgattr_schema);
+   if ($error)
+   {
+       die "Failed to form full tuple for pg_attribute: ", $error;
+   }
 }
 
 # Write a pg_attribute entry to postgres.bki
-sub bki_insert
+sub print_bki_insert
 {
    my $row        = shift;
    my @attnames   = @_;
    my $oid        = $row->{oid} ? "OID = $row->{oid} " : '';
-   my $bki_values = join ' ', map { $_ eq '' ? '""' : $_ } map $row->{$_},
-     @attnames;
+   my $bki_values = join ' ', @{$row}{@attnames};
    printf $bki "insert %s( %s )\n", $oid, $bki_values;
 }
 
+# Given a row reference, modify it so that it becomes a valid entry for
+# a catalog schema declaration in schemapg.h.
+#
 # The field values of a Schema_pg_xxx declaration are similar, but not
 # quite identical, to the corresponding values in postgres.bki.
-sub emit_schemapg_row
+sub morph_row_for_schemapg
 {
-   my $row        = shift;
-   my @bool_attrs = @_;
-
-   # Replace empty string by zero char constant
-   $row->{attidentity} ||= '\0';
-
-   # Supply appropriate quoting for these fields.
-   $row->{attname}     = q|{"| . $row->{attname} . q|"}|;
-   $row->{attstorage}  = q|'| . $row->{attstorage} . q|'|;
-   $row->{attalign}    = q|'| . $row->{attalign} . q|'|;
-   $row->{attidentity} = q|'| . $row->{attidentity} . q|'|;
-
-   # We don't emit initializers for the variable length fields at all.
-   # Only the fixed-size portions of the descriptors are ever used.
-   delete $row->{attacl};
-   delete $row->{attoptions};
-   delete $row->{attfdwoptions};
-
-   # Expand booleans from 'f'/'t' to 'false'/'true'.
-   # Some values might be other macros (eg FLOAT4PASSBYVAL), don't change.
-   foreach my $attr (@bool_attrs)
+   my $row           = shift;
+   my $pgattr_schema = shift;
+
+   foreach my $column (@$pgattr_schema)
    {
-       $row->{$attr} =
-           $row->{$attr} eq 't' ? 'true'
-         : $row->{$attr} eq 'f' ? 'false'
-         :                        $row->{$attr};
+       my $attname = $column->{name};
+       my $atttype = $column->{type};
+
+       # Some data types have special formatting rules.
+       if ($atttype eq 'name')
+       {
+           # add {" ... "} quoting
+           $row->{$attname} = sprintf(qq'{"%s"}', $row->{$attname});
+       }
+       elsif ($atttype eq 'char')
+       {
+           # Replace empty string by zero char constant; add single quotes
+           $row->{$attname} = '\0' if $row->{$attname} eq q|""|;
+           $row->{$attname} = sprintf("'%s'", $row->{$attname});
+       }
+
+       # Expand booleans from 'f'/'t' to 'false'/'true'.
+       # Some values might be other macros (eg FLOAT4PASSBYVAL),
+       # don't change.
+       elsif ($atttype eq 'bool')
+       {
+           $row->{$attname} = 'true' if $row->{$attname} eq 't';
+           $row->{$attname} = 'false' if $row->{$attname} eq 'f';
+       }
+
+       # We don't emit initializers for the variable length fields at all.
+       # Only the fixed-size portions of the descriptors are ever used.
+       delete $row->{$attname} if $column->{is_varlen};
    }
-   return $row;
 }
 
 sub usage
index 59b0f8ed5d6691d55861b18a5e9285af5bbd8249..96ac4025de2a10d86c28b5c14d69c57baf15c8ba 100644 (file)
@@ -31,6 +31,9 @@
 #define BKI_FORCE_NULL
 #define BKI_FORCE_NOT_NULL
 
+/* Specifies a default value for a catalog field */
+#define BKI_DEFAULT(value)
+
 /*
  * This is never defined; it's here only for documentation.
  *
index 6104254d7beb9d6e452467aa277d3cf60a6965c0..8159383834d26c25a36a4c013b551829c92e518f 100644 (file)
@@ -54,7 +54,7 @@ CATALOG(pg_attribute,1249) BKI_BOOTSTRAP BKI_WITHOUT_OIDS BKI_ROWTYPE_OID(75) BK
     * that no value has been explicitly set for this column, so ANALYZE
     * should use the default setting.
     */
-   int32       attstattarget;
+   int32       attstattarget BKI_DEFAULT(-1);
 
    /*
     * attlen is a copy of the typlen field from pg_type for this attribute.
@@ -90,7 +90,7 @@ CATALOG(pg_attribute,1249) BKI_BOOTSTRAP BKI_WITHOUT_OIDS BKI_ROWTYPE_OID(75) BK
     * descriptor, we may then update attcacheoff in the copies. This speeds
     * up the attribute walking process.
     */
-   int32       attcacheoff;
+   int32       attcacheoff BKI_DEFAULT(-1);
 
    /*
     * atttypmod records type-specific data supplied at table creation time
@@ -98,7 +98,7 @@ CATALOG(pg_attribute,1249) BKI_BOOTSTRAP BKI_WITHOUT_OIDS BKI_ROWTYPE_OID(75) BK
     * type-specific input and output functions as the third argument. The
     * value will generally be -1 for types that do not need typmod.
     */
-   int32       atttypmod;
+   int32       atttypmod BKI_DEFAULT(-1);
 
    /*
     * attbyval is a copy of the typbyval field from pg_type for this
@@ -131,13 +131,13 @@ CATALOG(pg_attribute,1249) BKI_BOOTSTRAP BKI_WITHOUT_OIDS BKI_ROWTYPE_OID(75) BK
    bool        attnotnull;
 
    /* Has DEFAULT value or not */
-   bool        atthasdef;
+   bool        atthasdef BKI_DEFAULT(f);
 
    /* One of the ATTRIBUTE_IDENTITY_* constants below, or '\0' */
-   char        attidentity;
+   char        attidentity BKI_DEFAULT("");
 
    /* Is dropped (ie, logically invisible) or not */
-   bool        attisdropped;
+   bool        attisdropped BKI_DEFAULT(f);
 
    /*
     * This flag specifies whether this column has ever had a local
@@ -148,10 +148,10 @@ CATALOG(pg_attribute,1249) BKI_BOOTSTRAP BKI_WITHOUT_OIDS BKI_ROWTYPE_OID(75) BK
     * not dropped by a parent's DROP COLUMN even if this causes the column's
     * attinhcount to become zero.
     */
-   bool        attislocal;
+   bool        attislocal BKI_DEFAULT(t);
 
    /* Number of times inherited from direct parent relation(s) */
-   int32       attinhcount;
+   int32       attinhcount BKI_DEFAULT(0);
 
    /* attribute's collation */
    Oid         attcollation;
@@ -160,13 +160,13 @@ CATALOG(pg_attribute,1249) BKI_BOOTSTRAP BKI_WITHOUT_OIDS BKI_ROWTYPE_OID(75) BK
    /* NOTE: The following fields are not present in tuple descriptors. */
 
    /* Column-level access permissions */
-   aclitem     attacl[1];
+   aclitem     attacl[1] BKI_DEFAULT(_null_);
 
    /* Column-level options */
-   text        attoptions[1];
+   text        attoptions[1] BKI_DEFAULT(_null_);
 
    /* Column-level FDW options */
-   text        attfdwoptions[1];
+   text        attfdwoptions[1] BKI_DEFAULT(_null_);
 #endif
 } FormData_pg_attribute;