From 74f52e4a3ecb58a6fb2f351d978576a67535c384 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat Date: Wed, 18 Jun 2025 16:56:25 +0530 Subject: [PATCH 1/2] Ignore temporary relations in RelidByRelfilenumber() Temporary relations may have the same relfilenumber as a permanent relation, or other temporary relations associated with other sessions. The function may get confused if the given relfilenumber and tablespace is associated with one or more temporary tables and throw an error. This will disturb the intended users of this function i.e. logical replication and autoprewarm, which lookup only permanent relation. Hence the function only considers only non-temporary relations. The third caller of this function is pg_filenode_relation(). It throws an error in such cases. We require proc number of the backend which created the temporary table along with the tablespace and relfilenumber in order to uniquely identify a temporary relation. Since any user visible backend identifier is not an argument to this function, it is doubtful whether the function was intended to be used to map temporary relations. In short, considering temporary relations seems to be an oversight in commit f01d1ae3a104019d6d68aeff85c4816a275130b3. Fix it. Author: Vignesh C Reported-By: Shenhao Wang Reviewed-By: Michael Paquier , Reviewed-By: Robert Haas , Reviewed-By: Kyotaro Horiguchi , Reviewed-By: Takamichi Osumi Reviewed-By: Ashutosh Bapat Discussion: https://www.postgresql.org/message-id/flat/bbaaf9f9-ebb2-645f-54bb-34d6efc7ac42%40fujitsu.com --- doc/src/sgml/func/func-admin.sgml | 12 ++++++------ src/backend/utils/adt/dbsize.c | 5 +++-- src/backend/utils/cache/relfilenumbermap.c | 20 +++++++++++++++++--- src/include/catalog/pg_proc.dat | 2 +- src/test/regress/expected/alter_table.out | 9 ++++++--- src/test/regress/expected/create_table.out | 10 ++++++++++ src/test/regress/sql/alter_table.sql | 10 ++++++---- src/test/regress/sql/create_table.sql | 6 ++++++ 8 files changed, 55 insertions(+), 19 deletions(-) diff --git a/doc/src/sgml/func/func-admin.sgml b/doc/src/sgml/func/func-admin.sgml index 446fdfe56f4..b59a3a9c52b 100644 --- a/doc/src/sgml/func/func-admin.sgml +++ b/doc/src/sgml/func/func-admin.sgml @@ -1829,12 +1829,12 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset regclass - Returns a relation's OID given the tablespace OID and filenode it is - stored under. This is essentially the inverse mapping of - pg_relation_filepath. For a relation in the - database's default tablespace, the tablespace can be specified as zero. - Returns NULL 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 + pg_relation_filepath only for permanent relations. + For a relation in the database's default tablespace, the tablespace can + be specified as zero. Returns NULL if no permanent + relation in the current database is associated with the given values. diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c index 25865b660ef..334ba6557ac 100644 --- a/src/backend/utils/adt/dbsize.c +++ b/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. * diff --git a/src/backend/utils/cache/relfilenumbermap.c b/src/backend/utils/cache/relfilenumbermap.c index 8a2f6f8c693..14c30276e24 100644 --- a/src/backend/utils/cache/relfilenumbermap.c +++ b/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", diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 118d6da1ace..ab1405aa0a5 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -7787,7 +7787,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' }, diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 08984dd98f1..d6d7cae73c9 100644 --- a/src/test/regress/expected/alter_table.out +++ b/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 diff --git a/src/test/regress/expected/create_table.out b/src/test/regress/expected/create_table.out index 76604705a93..dcdd59175da 100644 --- a/src/test/regress/expected/create_table.out +++ b/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)); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index fc6e36d0e78..22b21a51101 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/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; diff --git a/src/test/regress/sql/create_table.sql b/src/test/regress/sql/create_table.sql index 37a227148e9..d5fe0f861a9 100644 --- a/src/test/regress/sql/create_table.sql +++ b/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)); base-commit: 783cbb6d5e8bdf87d321286f210983c177ead967 -- 2.34.1