[CF 4720] "unexpected duplicate for tablespace" problem in logical replication

This branch was automatically generated by a robot using patches from an
email thread registered at:

https://commitfest.postgresql.org/patch/4720

The branch will be overwritten each time a new patch version is posted to
the thread, and also periodically to check for bitrot caused by changes
on the master branch.

Patch(es): https://www.postgresql.org/message-id/CAExHW5vaHu=s6eopJ2jxyGNRG--uEsJwYk92yxa2L_80+Y=1YA@mail.gmail.com
Author(s): Shenhao Wang

Author: Commitfest Bot

doc/src/sgml/func.sgml

@@ -30317,16 +30317,18 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset
         <indexterm>
          <primary>pg_filenode_relation</primary>
         </indexterm>
-        <function>pg_filenode_relation</function> ( <parameter>tablespace</parameter> <type>oid</type>, <parameter>filenode</parameter> <type>oid</type> )
+        <function>pg_filenode_relation</function>
+        ( <parameter>tablespace</parameter> <type>oid</type>,
+        <parameter>filenode</parameter> <type>oid</type>)
         <returnvalue>regclass</returnvalue>
        </para>
        <para>
-        Returns a relation's OID given the tablespace OID and filenode it is
-        stored under.  This is essentially the inverse mapping of
-        <function>pg_relation_filepath</function>.  For a relation in the
-        database's default tablespace, the tablespace can be specified as zero.
-        Returns <literal>NULL</literal> if no relation in the current database
-        is associated with the given values.
+        Returns a permanent relation's OID given the tablespace OID and filenode
+        it is stored under. This is essentially the inverse mapping of
+        <function>pg_relation_filepath</function> only for permanent relations.
+        For a relation in the database's default tablespace, the tablespace can
+        be specified as zero.  Returns <literal>NULL</literal> if no permanent
+        relation in the current database is associated with the given values.
        </para></entry>
       </row>
      </tbody>

src/backend/utils/adt/dbsize.c

@@ -933,8 +933,9 @@ pg_relation_filenode(PG_FUNCTION_ARGS)
  *
  * This is expected to be used when somebody wants to match an individual file
  * on the filesystem back to its table. That's not trivially possible via
- * pg_class, because that doesn't contain the relfilenumbers of shared and nailed
- * tables.
+ * pg_class, because that doesn't contain the relfilenumbers of shared and
+ * nailed tables. This function looks up only permanent relations; see
+ * RelidByRelfilenumber() for reasons.
  *
  * We don't fail but return NULL if we cannot find a mapping.
  *

src/backend/utils/cache/relfilenumbermap.c

@@ -127,10 +127,21 @@ InitializeRelfilenumberMap(void)
 }
 
 /*
- * Map a relation's (tablespace, relfilenumber) to a relation's oid and cache
- * the result.
+ * Map a relation's (tablespace, relfilenumber) to a relation's oid and cache the
+ * result.
  *
- * Returns InvalidOid if no relation matching the criteria could be found.
+ * A temporary relation may share its relfilenumber with a permanent relation,
+ * or other temporary relations. When looking up a permanent relation, this
+ * function may get confused by temporary relations which have same
+ * relfilenumber and throw an error. This behaviour may disturb logical
+ * replication and autoprewarm, both of which lookup permanent relations.
+ * Further in order to uniquely identify a temporary relation we also need the
+ * proc number of the backend which created it. The proc number is not available
+ * to this function. Given the intended use of this function, ignore temporary
+ * relations.
+ *
+ * Returns InvalidOid if no permanent relation matching the criteria could be
+ * found.
  */
 Oid
 RelidByRelfilenumber(Oid reltablespace, RelFileNumber relfilenumber)
@@ -208,6 +219,9 @@ RelidByRelfilenumber(Oid reltablespace, RelFileNumber relfilenumber)
 		{
 			Form_pg_class classform = (Form_pg_class) GETSTRUCT(ntp);
 
+			if (classform->relpersistence == RELPERSISTENCE_TEMP)
+				continue;
+
 			if (found)
 				elog(ERROR,
 					 "unexpected duplicate for tablespace %u, relfilenumber %u",

src/include/catalog/pg_proc.dat

@@ -7775,7 +7775,7 @@
 { oid => '2999', descr => 'filenode identifier of relation',
   proname => 'pg_relation_filenode', provolatile => 's', prorettype => 'oid',
   proargtypes => 'regclass', prosrc => 'pg_relation_filenode' },
-{ oid => '3454', descr => 'relation OID for filenode and tablespace',
+{ oid => '3454', descr => 'permanent relation OID for filenode and tablespace',
   proname => 'pg_filenode_relation', provolatile => 's',
   prorettype => 'regclass', proargtypes => 'oid oid',
   prosrc => 'pg_filenode_relation' },

src/test/regress/expected/alter_table.out

@@ -3565,14 +3565,17 @@ SELECT conname as constraint, obj_description(oid, 'pg_constraint') as comment F
 -- Check that we map relation oids to filenodes and back correctly.  Only
 -- display bad mappings so the test output doesn't change all the time.  A
 -- filenode function call can return NULL for a relation dropped concurrently
--- with the call's surrounding query, so ignore a NULL mapped_oid for
--- relations that no longer exist after all calls finish.
+-- with the call's surrounding query, so ignore a NULL mapped_oid for relations
+-- that no longer exist after all calls finish. The function maps only permanent
+-- relations, so ignore temporary relations.
 CREATE TEMP TABLE filenode_mapping AS
 SELECT
     oid, mapped_oid, reltablespace, relfilenode, relname
 FROM pg_class,
     pg_filenode_relation(reltablespace, pg_relation_filenode(oid)) AS mapped_oid
-WHERE relkind IN ('r', 'i', 'S', 't', 'm') AND mapped_oid IS DISTINCT FROM oid;
+WHERE relkind IN ('r', 'i', 'S', 't', 'm')
+  AND relpersistence != 't'
+  AND mapped_oid IS DISTINCT FROM oid;
 SELECT m.* FROM filenode_mapping m LEFT JOIN pg_class c ON c.oid = m.oid
 WHERE c.oid IS NOT NULL OR m.mapped_oid IS NOT NULL;
  oid | mapped_oid | reltablespace | relfilenode | relname 

src/test/regress/expected/create_table.out

@@ -102,6 +102,16 @@ ERROR:  tables declared WITH OIDS are not supported
 -- but explicitly not adding oids is still supported
 CREATE TEMP TABLE withoutoid() WITHOUT OIDS; DROP TABLE withoutoid;
 CREATE TEMP TABLE withoutoid() WITH (oids = false); DROP TABLE withoutoid;
+-- verify that temporary tables data are not retrieved by pg_filenode_relation
+CREATE TEMP TABLE filenoderelationcheck(c1 int);
+SELECT pg_filenode_relation (reltablespace, pg_relation_filenode(oid)) FROM pg_class
+WHERE relname = 'filenoderelationcheck';
+ pg_filenode_relation 
+----------------------
+ 
+(1 row)
+
+DROP TABLE filenoderelationcheck;
 -- check restriction with default expressions
 -- invalid use of column reference in default expressions
 CREATE TABLE default_expr_column (id int DEFAULT (id));

src/test/regress/sql/alter_table.sql

@@ -2200,15 +2200,17 @@ SELECT conname as constraint, obj_description(oid, 'pg_constraint') as comment F
 -- Check that we map relation oids to filenodes and back correctly.  Only
 -- display bad mappings so the test output doesn't change all the time.  A
 -- filenode function call can return NULL for a relation dropped concurrently
--- with the call's surrounding query, so ignore a NULL mapped_oid for
--- relations that no longer exist after all calls finish.
+-- with the call's surrounding query, so ignore a NULL mapped_oid for relations
+-- that no longer exist after all calls finish. The function maps only permanent
+-- relations, so ignore temporary relations.
 CREATE TEMP TABLE filenode_mapping AS
 SELECT
     oid, mapped_oid, reltablespace, relfilenode, relname
 FROM pg_class,
     pg_filenode_relation(reltablespace, pg_relation_filenode(oid)) AS mapped_oid
-WHERE relkind IN ('r', 'i', 'S', 't', 'm') AND mapped_oid IS DISTINCT FROM oid;
-
+WHERE relkind IN ('r', 'i', 'S', 't', 'm')
+  AND relpersistence != 't'
+  AND mapped_oid IS DISTINCT FROM oid;
 SELECT m.* FROM filenode_mapping m LEFT JOIN pg_class c ON c.oid = m.oid
 WHERE c.oid IS NOT NULL OR m.mapped_oid IS NOT NULL;
 

src/test/regress/sql/create_table.sql

@@ -68,6 +68,12 @@ CREATE TABLE withoid() WITH (oids = true);
 CREATE TEMP TABLE withoutoid() WITHOUT OIDS; DROP TABLE withoutoid;
 CREATE TEMP TABLE withoutoid() WITH (oids = false); DROP TABLE withoutoid;
 
+-- verify that temporary tables data are not retrieved by pg_filenode_relation
+CREATE TEMP TABLE filenoderelationcheck(c1 int);
+SELECT pg_filenode_relation (reltablespace, pg_relation_filenode(oid)) FROM pg_class
+WHERE relname = 'filenoderelationcheck';
+DROP TABLE filenoderelationcheck;
+
 -- check restriction with default expressions
 -- invalid use of column reference in default expressions
 CREATE TABLE default_expr_column (id int DEFAULT (id));