Remove pg_replication_origin's TOAST table.

nathan-bossart · nathan-bossart · commit 16bf24e0e471 · 2025-05-07T14:47:36.000-05:00
A few places that access this catalog don't set up an active snapshot before potentially accessing its TOAST table. However, roname (the replication origin name) is the only varlena column, so this is only a problem if the name requires out-of-line storage. This commit removes its TOAST table to avoid needing to set up a snapshot. It also places a limit on replication origin names so that attempts to set long names will fail with a more user-friendly error. Those chosen limit of 512 bytes should be sufficient to avoid "row is too big" errors independent of BLCKSZ, but it should also be lenient enough for all reasonable use-cases. Bumps catversion. Reviewed-by: Michael Paquier <michael@paquier.xyz> Reviewed-by: Amit Kapila <amit.kapila16@gmail.com> Reviewed-by: Euler Taveira <euler@eulerto.com> Reviewed-by: Nisha Moond <nisha.moond412@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/ZvMSUPOqUU-VNADN%40nathan
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
@@ -29940,6 +29940,7 @@ postgres=# SELECT '0/0'::pg_lsn + pd.segment_number * ps.setting::int + :offset
        <para>
         Creates a replication origin with the given external
         name, and returns the internal ID assigned to it.
+        The name must be no longer than 512 bytes.
        </para></entry>
       </row>
 
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
@@ -346,8 +346,6 @@ IsSharedRelation(Oid relationId)
 		relationId == PgDbRoleSettingToastIndex ||
 		relationId == PgParameterAclToastTable ||
 		relationId == PgParameterAclToastIndex ||
-		relationId == PgReplicationOriginToastTable ||
-		relationId == PgReplicationOriginToastIndex ||
 		relationId == PgShdescriptionToastTable ||
 		relationId == PgShdescriptionToastIndex ||
 		relationId == PgShseclabelToastTable ||
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
@@ -264,6 +264,18 @@ replorigin_create(const char *roname)
 	SysScanDesc scan;
 	ScanKeyData key;
 
+	/*
+	 * To avoid needing a TOAST table for pg_replication_origin, we limit
+	 * replication origin names to 512 bytes.  This should be more than enough
+	 * for all practical use.
+	 */
+	if (strlen(roname) > MAX_RONAME_LEN)
+		ereport(ERROR,
+				(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+				 errmsg("replication origin name is too long"),
+				 errdetail("Replication origin names must be no longer than %d bytes.",
+						   MAX_RONAME_LEN)));
+
 	roname_d = CStringGetTextDatum(roname);
 
 	Assert(IsTransactionState());
@@ -287,6 +299,17 @@ replorigin_create(const char *roname)
 
 	rel = table_open(ReplicationOriginRelationId, ExclusiveLock);
 
+	/*
+	 * We want to be able to access pg_replication_origin without setting up a
+	 * snapshot.  To make that safe, it needs to not have a TOAST table, since
+	 * TOASTed data cannot be fetched without a snapshot.  As of this writing,
+	 * its only varlena column is roname, which we limit to 512 bytes to avoid
+	 * needing out-of-line storage.  If you add a TOAST table to this catalog,
+	 * be sure to set up a snapshot everywhere it might be needed.  For more
+	 * information, see https://postgr.es/m/ZvMSUPOqUU-VNADN%40nathan.
+	 */
+	Assert(!OidIsValid(rel->rd_rel->reltoastrelid));
+
 	for (roident = InvalidOid + 1; roident < PG_UINT16_MAX; roident++)
 	{
 		bool		nulls[Natts_pg_replication_origin];
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	202504091
+#define CATALOG_VERSION_NO	202505071
 
 #endif
diff --git a/src/include/catalog/pg_replication_origin.h b/src/include/catalog/pg_replication_origin.h
@@ -54,8 +54,6 @@ CATALOG(pg_replication_origin,6000,ReplicationOriginRelationId) BKI_SHARED_RELAT
 
 typedef FormData_pg_replication_origin *Form_pg_replication_origin;
 
-DECLARE_TOAST_WITH_MACRO(pg_replication_origin, 4181, 4182, PgReplicationOriginToastTable, PgReplicationOriginToastIndex);
-
 DECLARE_UNIQUE_INDEX_PKEY(pg_replication_origin_roiident_index, 6001, ReplicationOriginIdentIndex, pg_replication_origin, btree(roident oid_ops));
 DECLARE_UNIQUE_INDEX(pg_replication_origin_roname_index, 6002, ReplicationOriginNameIndex, pg_replication_origin, btree(roname text_ops));
 
diff --git a/src/include/replication/origin.h b/src/include/replication/origin.h
@@ -33,6 +33,13 @@ typedef struct xl_replorigin_drop
 #define InvalidRepOriginId 0
 #define DoNotReplicateId PG_UINT16_MAX
 
+/*
+ * To avoid needing a TOAST table for pg_replication_origin, we limit
+ * replication origin names to 512 bytes.  This should be more than enough for
+ * all practical use.
+ */
+#define MAX_RONAME_LEN	512
+
 extern PGDLLIMPORT RepOriginId replorigin_session_origin;
 extern PGDLLIMPORT XLogRecPtr replorigin_session_origin_lsn;
 extern PGDLLIMPORT TimestampTz replorigin_session_origin_timestamp;
diff --git a/src/test/regress/expected/misc_functions.out b/src/test/regress/expected/misc_functions.out
@@ -914,3 +914,7 @@ SELECT test_relpath();
  
 (1 row)
 
+-- pg_replication_origin.roname limit
+SELECT pg_replication_origin_create('regress_' || repeat('a', 505));
+ERROR:  replication origin name is too long
+DETAIL:  Replication origin names must be no longer than 512 bytes.
diff --git a/src/test/regress/expected/misc_sanity.out b/src/test/regress/expected/misc_sanity.out
@@ -42,6 +42,11 @@ WHERE refclassid = 0 OR refobjid = 0 OR
 -- as user data by pg_upgrade, which would cause failures.
 -- 3. pg_authid, since its toast table cannot be accessed when it would be
 -- needed, i.e., during authentication before we've selected a database.
+-- 4. pg_replication_origin, since we want to be able to access that catalog
+-- without setting up a snapshot.  To make that safe, it needs to not have a
+-- toast table, since toasted data cannot be fetched without a snapshot.  As of
+-- this writing, its only varlena column is roname, which we limit to 512 bytes
+-- to avoid needing out-of-line storage.
 SELECT relname, attname, atttypid::regtype
 FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid
 WHERE c.oid < 16384 AND
@@ -61,7 +66,8 @@ ORDER BY 1, 2;
  pg_class                | relpartbound  | pg_node_tree
  pg_largeobject          | data          | bytea
  pg_largeobject_metadata | lomacl        | aclitem[]
-(10 rows)
+ pg_replication_origin   | roname        | text
+(11 rows)
 
 -- system catalogs without primary keys
 --
diff --git a/src/test/regress/sql/misc_functions.sql b/src/test/regress/sql/misc_functions.sql
@@ -411,3 +411,6 @@ CREATE FUNCTION test_relpath()
     AS :'regresslib'
     LANGUAGE C;
 SELECT test_relpath();
+
+-- pg_replication_origin.roname limit
+SELECT pg_replication_origin_create('regress_' || repeat('a', 505));
diff --git a/src/test/regress/sql/misc_sanity.sql b/src/test/regress/sql/misc_sanity.sql
@@ -45,6 +45,11 @@ WHERE refclassid = 0 OR refobjid = 0 OR
 -- as user data by pg_upgrade, which would cause failures.
 -- 3. pg_authid, since its toast table cannot be accessed when it would be
 -- needed, i.e., during authentication before we've selected a database.
+-- 4. pg_replication_origin, since we want to be able to access that catalog
+-- without setting up a snapshot.  To make that safe, it needs to not have a
+-- toast table, since toasted data cannot be fetched without a snapshot.  As of
+-- this writing, its only varlena column is roname, which we limit to 512 bytes
+-- to avoid needing out-of-line storage.
 
 SELECT relname, attname, atttypid::regtype
 FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid
