Re: assert pg_class.relnatts is consistent

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2020-Feb-14, John Naylor wrote:
>> One possible objection to what I wrote above is that it adds a
>> different kind of special case, but in a sneaky way. Perhaps it would
>> be more principled to treat it the same as oid after all. If we do
>> that, it would help to add a comment that we can't treat relnatts like
>> pronangs, since we need more information than what's in each pg_class
>> row.

> How about something like this? (untested)

I think John's idea of setting a dummy BKI_DEFAULT value is better,
as that means the only code that has to worry about this directly
is the code that's actually filling in relnatts.  As far as said
code goes, we don't need an additional global variable when we can
just look in the $catalogs data structure; and I'm not a fan of
cramming this into the OID-assignment logic just to save a loop.
So that leads me to the attached.

(I agree with Alvaro's thought of shortening AddDefaultValues,
but didn't do that here.)

            regards, tom lane

diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index 8032512..ad24f4d 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -184,6 +184,15 @@ my $PG_CATALOG_NAMESPACE =
     'PG_CATALOG_NAMESPACE');


+# Fill in pg_class.relnatts by looking at the referenced catalog's schema.
+# This is ugly but there's no better place; Catalog::AddDefaultValues
+# can't do it, for lack of easy access to the other catalog.
+foreach my $row (@{ $catalog_data{pg_class} })
+{
+    $row->{relnatts} = scalar(@{ $catalogs{ $row->{relname} }->{columns} });
+}
+
+
 # Build lookup tables.

 # access method OID lookup
diff --git a/src/include/catalog/pg_class.dat b/src/include/catalog/pg_class.dat
index f70d5ba..b28bba7 100644
--- a/src/include/catalog/pg_class.dat
+++ b/src/include/catalog/pg_class.dat
@@ -14,51 +14,15 @@

 # Note: only bootstrap catalogs, ie those marked BKI_BOOTSTRAP, need to
 # have entries here.  Be sure that the OIDs listed here match those given in
-# their CATALOG and BKI_ROWTYPE_OID macros, and that the relnatts values are
-# correct.
-
-# Note: "3" in the relfrozenxid column stands for FirstNormalTransactionId;
-# similarly, "1" in relminmxid stands for FirstMultiXactId
+# their CATALOG and BKI_ROWTYPE_OID macros.

 { oid => '1247',
-  relname => 'pg_type', reltype => 'pg_type', relam => 'heap',
-  relfilenode => '0', relpages => '0', reltuples => '0', relallvisible => '0',
-  reltoastrelid => '0', relhasindex => 'f', relisshared => 'f',
-  relpersistence => 'p', relkind => 'r', relnatts => '31', relchecks => '0',
-  relhasrules => 'f', relhastriggers => 'f', relhassubclass => 'f',
-  relrowsecurity => 'f', relforcerowsecurity => 'f', relispopulated => 't',
-  relreplident => 'n', relispartition => 'f', relfrozenxid => '3',
-  relminmxid => '1', relacl => '_null_', reloptions => '_null_',
-  relpartbound => '_null_' },
+  relname => 'pg_type', reltype => 'pg_type' },
 { oid => '1249',
-  relname => 'pg_attribute', reltype => 'pg_attribute', relam => 'heap',
-  relfilenode => '0', relpages => '0', reltuples => '0', relallvisible => '0',
-  reltoastrelid => '0', relhasindex => 'f', relisshared => 'f',
-  relpersistence => 'p', relkind => 'r', relnatts => '25', relchecks => '0',
-  relhasrules => 'f', relhastriggers => 'f', relhassubclass => 'f',
-  relrowsecurity => 'f', relforcerowsecurity => 'f', relispopulated => 't',
-  relreplident => 'n', relispartition => 'f', relfrozenxid => '3',
-  relminmxid => '1', relacl => '_null_', reloptions => '_null_',
-  relpartbound => '_null_' },
+  relname => 'pg_attribute', reltype => 'pg_attribute' },
 { oid => '1255',
-  relname => 'pg_proc', reltype => 'pg_proc', relam => 'heap',
-  relfilenode => '0', relpages => '0', reltuples => '0', relallvisible => '0',
-  reltoastrelid => '0', relhasindex => 'f', relisshared => 'f',
-  relpersistence => 'p', relkind => 'r', relnatts => '29', relchecks => '0',
-  relhasrules => 'f', relhastriggers => 'f', relhassubclass => 'f',
-  relrowsecurity => 'f', relforcerowsecurity => 'f', relispopulated => 't',
-  relreplident => 'n', relispartition => 'f', relfrozenxid => '3',
-  relminmxid => '1', relacl => '_null_', reloptions => '_null_',
-  relpartbound => '_null_' },
+  relname => 'pg_proc', reltype => 'pg_proc' },
 { oid => '1259',
-  relname => 'pg_class', reltype => 'pg_class', relam => 'heap',
-  relfilenode => '0', relpages => '0', reltuples => '0', relallvisible => '0',
-  reltoastrelid => '0', relhasindex => 'f', relisshared => 'f',
-  relpersistence => 'p', relkind => 'r', relnatts => '33', relchecks => '0',
-  relhasrules => 'f', relhastriggers => 'f', relhassubclass => 'f',
-  relrowsecurity => 'f', relforcerowsecurity => 'f', relispopulated => 't',
-  relreplident => 'n', relispartition => 'f', relfrozenxid => '3',
-  relminmxid => '1', relacl => '_null_', reloptions => '_null_',
-  relpartbound => '_null_' },
+  relname => 'pg_class', reltype => 'pg_class' },

 ]
diff --git a/src/include/catalog/pg_class.h b/src/include/catalog/pg_class.h
index a12fc1f..78b33b2 100644
--- a/src/include/catalog/pg_class.h
+++ b/src/include/catalog/pg_class.h
@@ -24,6 +24,9 @@
 /* ----------------
  *        pg_class definition.  cpp turns this into
  *        typedef struct FormData_pg_class
+ *
+ * Note that the BKI_DEFAULT values below are only used for rows describing
+ * BKI_BOOTSTRAP catalogs, since only those rows appear in pg_class.dat.
  * ----------------
  */
 CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,RelationRelation_Rowtype_Id)
BKI_SCHEMA_MACRO
@@ -47,41 +50,41 @@ CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,Relat
     Oid            relowner BKI_DEFAULT(PGUID);

     /* access method; 0 if not a table / index */
-    Oid            relam BKI_LOOKUP(pg_am);
+    Oid            relam BKI_DEFAULT(heap) BKI_LOOKUP(pg_am);

     /* identifier of physical storage file */
     /* relfilenode == 0 means it is a "mapped" relation, see relmapper.c */
-    Oid            relfilenode;
+    Oid            relfilenode BKI_DEFAULT(0);

     /* identifier of table space for relation (0 means default for database) */
     Oid            reltablespace BKI_DEFAULT(0) BKI_LOOKUP(pg_tablespace);

     /* # of blocks (not always up-to-date) */
-    int32        relpages;
+    int32        relpages BKI_DEFAULT(0);

     /* # of tuples (not always up-to-date) */
-    float4        reltuples;
+    float4        reltuples BKI_DEFAULT(0);

     /* # of all-visible blocks (not always up-to-date) */
-    int32        relallvisible;
+    int32        relallvisible BKI_DEFAULT(0);

     /* OID of toast table; 0 if none */
-    Oid            reltoastrelid;
+    Oid            reltoastrelid BKI_DEFAULT(0);

     /* T if has (or has had) any indexes */
-    bool        relhasindex;
+    bool        relhasindex BKI_DEFAULT(f);

     /* T if shared across databases */
-    bool        relisshared;
+    bool        relisshared BKI_DEFAULT(f);

     /* see RELPERSISTENCE_xxx constants below */
-    char        relpersistence;
+    char        relpersistence BKI_DEFAULT(p);

     /* see RELKIND_xxx constants below */
-    char        relkind;
+    char        relkind BKI_DEFAULT(r);

     /* number of user attributes */
-    int16        relnatts;
+    int16        relnatts BKI_DEFAULT(0);    /* genbki.pl will fill this in */

     /*
      * Class pg_attribute must contain exactly "relnatts" user attributes
@@ -90,51 +93,51 @@ CATALOG(pg_class,1259,RelationRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(83,Relat
      */

     /* # of CHECK constraints for class */
-    int16        relchecks;
+    int16        relchecks BKI_DEFAULT(0);

     /* has (or has had) any rules */
-    bool        relhasrules;
+    bool        relhasrules BKI_DEFAULT(f);

     /* has (or has had) any TRIGGERs */
-    bool        relhastriggers;
+    bool        relhastriggers BKI_DEFAULT(f);

     /* has (or has had) child tables or indexes */
-    bool        relhassubclass;
+    bool        relhassubclass BKI_DEFAULT(f);

     /* row security is enabled or not */
-    bool        relrowsecurity;
+    bool        relrowsecurity BKI_DEFAULT(f);

     /* row security forced for owners or not */
-    bool        relforcerowsecurity;
+    bool        relforcerowsecurity BKI_DEFAULT(f);

     /* matview currently holds query results */
-    bool        relispopulated;
+    bool        relispopulated BKI_DEFAULT(t);

     /* see REPLICA_IDENTITY_xxx constants */
-    char        relreplident;
+    char        relreplident BKI_DEFAULT(n);

     /* is relation a partition? */
-    bool        relispartition;
+    bool        relispartition BKI_DEFAULT(f);

     /* heap for rewrite during DDL, link to original rel */
     Oid            relrewrite BKI_DEFAULT(0);

     /* all Xids < this are frozen in this rel */
-    TransactionId relfrozenxid;
+    TransactionId relfrozenxid BKI_DEFAULT(3);    /* FirstNormalTransactionId */

     /* all multixacts in this rel are >= this; it is really a MultiXactId */
-    TransactionId relminmxid;
+    TransactionId relminmxid BKI_DEFAULT(1);    /* FirstMultiXactId */

 #ifdef CATALOG_VARLEN            /* variable-length fields start here */
     /* NOTE: These fields are not present in a relcache entry's rd_rel field. */
     /* access permissions */
-    aclitem        relacl[1];
+    aclitem        relacl[1] BKI_DEFAULT(_null_);

     /* access-method-specific options */
-    text        reloptions[1];
+    text        reloptions[1] BKI_DEFAULT(_null_);

     /* partition bound node tree */
-    pg_node_tree relpartbound;
+    pg_node_tree relpartbound BKI_DEFAULT(_null_);
 #endif
 } FormData_pg_class;