On Wed, Apr 23, 2025 at 04:44:51PM -0500, Nathan Bossart wrote: > On Wed, Apr 23, 2025 at 05:33:41PM -0400, Tom Lane wrote: >> I don't see any comments in this patch that capture the real >> reason for removing pg_replication_origin's TOAST table, >> namely (IIUC) that we'd like to be able to access that catalog >> without a snapshot. I think it's important to record that >> knowledge, because otherwise somebody is likely to think they >> can undo this change for $whatever-reason. > > D'oh, yes, I'd better add that.
I updated the comment atop the check in the misc_sanity test for system tables with varlena columns but no toast table. If you were to reintroduce pg_replication_origin's toast table, you'd have to at least fix the expected output for this test, so the comment theoretically has a higher chance of being seen. -- nathan
>From 2d7e89e4ec49b1156b33b11219dbf403aefc5466 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@postgresql.org> Date: Mon, 28 Apr 2025 16:27:33 -0500 Subject: [PATCH v5 1/1] Remove pg_replication_origin's TOAST table. Reviewed-by: Michael Paquier <mich...@paquier.xyz> Reviewed-by: Amit Kapila <amit.kapil...@gmail.com> Reviewed-by: Euler Taveira <eu...@eulerto.com> Reviewed-by: Nisha Moond <nisha.moond...@gmail.com> Reviewed-by: Tom Lane <t...@sss.pgh.pa.us> Discussion: https://postgr.es/m/ZvMSUPOqUU-VNADN%40nathan --- doc/src/sgml/func.sgml | 1 + src/backend/catalog/catalog.c | 2 -- src/backend/replication/logical/origin.c | 12 ++++++++++++ src/include/catalog/pg_replication_origin.h | 2 -- src/include/replication/origin.h | 7 +++++++ src/test/regress/expected/misc_functions.out | 4 ++++ src/test/regress/expected/misc_sanity.out | 7 ++++++- src/test/regress/sql/misc_functions.sql | 3 +++ src/test/regress/sql/misc_sanity.sql | 4 ++++ 9 files changed, 37 insertions(+), 5 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 574a544d9fa..593e45495be 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -29941,6 +29941,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 index 60000bd0bc7..59caae8f1bc 100644 --- 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 index 6583dd497da..e9629350cb0 100644 --- a/src/backend/replication/logical/origin.c +++ b/src/backend/replication/logical/origin.c @@ -1296,6 +1296,18 @@ pg_replication_origin_create(PG_FUNCTION_ARGS) elog(WARNING, "replication origins created by regression test cases should have names starting with \"regress_\""); #endif + /* + * To avoid needing a TOAST table for pg_replication_origin, we restrict + * replication origin names to 512 bytes. This should be more than enough + * for all practical use. + */ + if (strlen(name) > 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))); + roident = replorigin_create(name); pfree(name); diff --git a/src/include/catalog/pg_replication_origin.h b/src/include/catalog/pg_replication_origin.h index deb43065fe9..7ade8bbda39 100644 --- 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 index 9cb2248fa9f..29724c0dcae 100644 --- 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 restrict + * 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 index d3f5d16a672..3de32c3c7ef 100644 --- 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 length restriction +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 index b032a3f4761..b0b76fe3d9a 100644 --- a/src/test/regress/expected/misc_sanity.out +++ b/src/test/regress/expected/misc_sanity.out @@ -42,6 +42,10 @@ 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. As of this writing, its toast table would +-- only be used for replication origin names, which we can avoid with a +-- reasonable length restriction. SELECT relname, attname, atttypid::regtype FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid WHERE c.oid < 16384 AND @@ -61,7 +65,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 index aaebb298330..ea72a62dab7 100644 --- 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 length restriction +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 index 135793871b4..86c193a0c42 100644 --- a/src/test/regress/sql/misc_sanity.sql +++ b/src/test/regress/sql/misc_sanity.sql @@ -45,6 +45,10 @@ 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. As of this writing, its toast table would +-- only be used for replication origin names, which we can avoid with a +-- reasonable length restriction. SELECT relname, attname, atttypid::regtype FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid -- 2.39.5 (Apple Git-154)
