Don't create relfilenode for relations without storage

Some relation kinds had relfilenode set to some non-zero value, but
apparently the actual files did not really exist because creation was
prevented elsewhere.  Get rid of the phony pg_class.relfilenode values.

Catversion bumped, but only because the sanity_test check will fail if
run in a system initdb'd with the previous version.

Reviewed-by: Kyotaro HORIGUCHI, Michael Paquier
Discussion: https://postgr.es/m/20181206215552.fm2ypuxq6nhpwjuc@alvherre.pgsql

diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 694000798a7c5bd456e873db45c9f681e3f39cbb..472285d3913eebe0afd3184bbdbb64f9acc99095 100644 (file)
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -324,35 +324,25 @@ heap_create(const char *relname,
                        get_namespace_name(relnamespace), relname),
                 errdetail("System catalog modifications are currently disallowed.")));
 
-   /*
-    * Decide if we need storage or not, and handle a couple other special
-    * cases for particular relkinds.
-    */
+   /* Handle reltablespace for specific relkinds. */
    switch (relkind)
    {
        case RELKIND_VIEW:
        case RELKIND_COMPOSITE_TYPE:
        case RELKIND_FOREIGN_TABLE:
-           create_storage = false;
 
            /*
             * Force reltablespace to zero if the relation has no physical
             * storage.  This is mainly just for cleanliness' sake.
+            *
+            * Partitioned tables and indexes don't have physical storage
+            * either, but we want to keep their tablespace settings so that
+            * their children can inherit it.
             */
            reltablespace = InvalidOid;
            break;
 
-       case RELKIND_PARTITIONED_TABLE:
-       case RELKIND_PARTITIONED_INDEX:
-           /*
-            * For partitioned tables and indexes, preserve tablespace so that
-            * it's used as the tablespace for future partitions.
-            */
-           create_storage = false;
-           break;
-
        case RELKIND_SEQUENCE:
-           create_storage = true;
 
            /*
             * Force reltablespace to zero for sequences, since we don't
@@ -361,19 +351,21 @@ heap_create(const char *relname,
            reltablespace = InvalidOid;
            break;
        default:
-           create_storage = true;
            break;
    }
 
    /*
-    * Unless otherwise requested, the physical ID (relfilenode) is initially
-    * the same as the logical ID (OID).  When the caller did specify a
-    * relfilenode, it already exists; do not attempt to create it.
+    * Decide whether to create storage. If caller passed a valid relfilenode,
+    * storage is already created, so don't do it here.  Also don't create it
+    * for relkinds without physical storage.
     */
-   if (OidIsValid(relfilenode))
+   if (!RELKIND_HAS_STORAGE(relkind) || OidIsValid(relfilenode))
        create_storage = false;
    else
+   {
+       create_storage = true;
        relfilenode = relid;
+   }
 
    /*
     * Never allow a pg_class entry to explicitly specify the database's

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index b5f5a224171bc2abe7331f8913d932796e3aa9c1..06503bc98b2a50d23c8d2be8174082cafc1c83d8 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1253,6 +1253,10 @@ RelationBuildDesc(Oid targetRelId, bool insertIt)
 static void
 RelationInitPhysicalAddr(Relation relation)
 {
+   /* these relations kinds never have storage */
+   if (!RELKIND_HAS_STORAGE(relation->rd_rel->relkind))
+       return;
+
    if (relation->rd_rel->reltablespace)
        relation->rd_node.spcNode = relation->rd_rel->reltablespace;
    else

diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index d2a49b04f407cfb3c160cc8bf2342cc110f508e4..7b3c88a138564d4379f75a60b9e493db3ab67ae8 100644 (file)
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*                         yyyymmddN */
-#define CATALOG_VERSION_NO 201901031
+#define CATALOG_VERSION_NO 201901041
 
 #endif

diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index a7c3aa95c27a9ddceab33405ea765dfc1467e232..bc20950cc8ac52fa7843637c48a8c78a8a3301f6 100644 (file)
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -450,13 +450,12 @@ typedef struct ViewOptions
 
 /*
  * RelationIsMapped
- *     True if the relation uses the relfilenode map.
- *
- * NB: this is only meaningful for relkinds that have storage, else it
- * will misleadingly say "true".
+ *     True if the relation uses the relfilenode map.  Note multiple eval
+ *     of argument!
  */
 #define RelationIsMapped(relation) \
-   ((relation)->rd_rel->relfilenode == InvalidOid)
+   (RELKIND_HAS_STORAGE((relation)->rd_rel->relkind) && \
+    ((relation)->rd_rel->relfilenode == InvalidOid))
 
 /*
  * RelationOpenSmgr

diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out
index 009a89fc1ad4d6d4247bdd8de4ab363c679c4e5e..89537bcfcf4a6cd914fbeced27bee3c9e4d97dbf 100644 (file)
--- a/src/test/regress/expected/sanity_check.out
+++ b/src/test/regress/expected/sanity_check.out
@@ -224,3 +224,12 @@ SELECT relname, nspname
 ---------+---------
 (0 rows)
 
+-- check that relations without storage don't have relfilenode
+SELECT relname, relkind
+  FROM pg_class
+ WHERE relkind IN ('v', 'c', 'f', 'p', 'I')
+       AND relfilenode <> 0;
+ relname | relkind 
+---------+---------
+(0 rows)
+

diff --git a/src/test/regress/sql/sanity_check.sql b/src/test/regress/sql/sanity_check.sql
index a2feebc91bff9d93211aa0817ba67f5de0cc1e23..a4ec00309ffb41a2f5712c03178c5f441c43c35d 100644 (file)
--- a/src/test/regress/sql/sanity_check.sql
+++ b/src/test/regress/sql/sanity_check.sql
@@ -31,3 +31,9 @@ SELECT relname, nspname
      AND NOT EXISTS (SELECT 1 FROM pg_index i WHERE indrelid = c.oid
                      AND indkey[0] = a.attnum AND indnatts = 1
                      AND indisunique AND indimmediate);
+
+-- check that relations without storage don't have relfilenode
+SELECT relname, relkind
+  FROM pg_class
+ WHERE relkind IN ('v', 'c', 'f', 'p', 'I')
+       AND relfilenode <> 0;