Re: Proposal: Conflict log history table for Logical Replication

.

On Thu, May 14, 2026 at 2:23 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Mon, 11 May 2026 at 14:59, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
> >
> > I started reviewing the patches.
> > Here are minor comments for 0001 patch:
> >
> > 1. If allow_system_table_mods=on we can add/drop columns of conflict log tables
> > But the same for pg_toast or other catalog tables are prohibited. Also
> > for other system tables we are getting following error.
> >
> > postgres=# ALTER TABLE pg_toast.pg_toast_16413 DROP COLUMN chunk_seq;
> > ERROR: ALTER action DROP COLUMN cannot be performed on relation
> > "pg_toast_16413"
> >
> > DETAIL: This operation is not supported for TOAST tables.
> > postgres=# ALTER TABLE pg_publication DROP COLUMN pubname;
> > ERROR: cannot drop column pubname of table pg_publication because it
> > is required by the database system
> > postgres=# ALTER TABLE pg_description DROP COLUMN description;
> > ERROR: cannot drop column description of table pg_description because
> > it is required by the database system
> >
> > postgres=# ALTER TABLE pg_conflict.pg_conflict_log_16408 DROP COLUMN relname;
> > ALTER TABLE
> >
> > Should we prohibit it for conflict log tables as well?
>
> The reason it fails for regular system catalogs is that
> IsPinnedObject() returns true for them. Objects with OIDs less than
> FirstUnpinnedObjectId(12000) are considered pinned, which includes the
> core catalogs created during initdb. In such cases, PostgreSQL
> immediately throws the following error:
> /*
> * If the target object is pinned, we can just error out immediately; it
> * won't have any objects recorded as depending on it.
> */
> if (IsPinnedObject(object->classId, object->objectId))
> ereport(ERROR,
> (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
> errmsg("cannot drop %s because it is required by the
> database system",
> getObjectDescription(object, false))));
> The call chain is:
> ATExecDropColumn -> performMultipleDeletions -> findDependentObjects
> -> IsPinnedObject
>
> However, the conflict log tables are not created during initdb; they
> are created later during subscription creation. Therefore, they are
> not considered pinned objects, IsPinnedObject() returns false, and the
> DROP COLUMN operation is allowed.
>
> I also noticed that ADD COLUMN is currently allowed on system tables
> when allow_system_table_mods is enabled:
> postgres=# SET allow_system_table_mods = on;
> SET
> postgres=# ALTER TABLE pg_description ADD COLUMN fake text;
> ALTER TABLE
>
> There are also cases where such operations lead to assertion failures.
> For example:
> postgres=# SET allow_system_table_mods = on;
> SET
> postgres=# alter table pg_type add column fake int;
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Failed.
> The connection to the server was lost. Attempting reset: Failed.
>
> TRAP: failed Assert("i >= 0 && i < tupdesc->natts"), File:
> "../../../src/include/access/tupdesc.h", Line: 182, PID: 11443
> postgres: vignesh postgres [local] ALTER
> TABLE(ExceptionalCondition+0xba) [0x616a67fc753c]
> postgres: vignesh postgres [local] ALTER TABLE(+0x7057fa) [0x616a67d067fa]
> postgres: vignesh postgres [local] ALTER
> TABLE(build_column_default+0x34) [0x616a67d08961]
> postgres: vignesh postgres [local] ALTER TABLE(+0x3e8875) [0x616a679e9875]
> postgres: vignesh postgres [local] ALTER TABLE(+0x3e34e8) [0x616a679e44e8]
> postgres: vignesh postgres [local] ALTER TABLE(+0x3e2e24) [0x616a679e3e24]
>
> The documentation also explicitly warns about this behavior at [1]:
> Allows modification of the structure of system tables as well as
> certain other risky actions on system tables. This is otherwise not
> allowed even for superusers. Ill-advised use of this setting can cause
> irretrievable data loss or seriously corrupt the database system.
>
> Given this, I am not sure whether we should specifically prevent
> dropping columns from conflict log tables when allow_system_table_mods
> is enabled.
>

+1. We can keep the current behavior as-is since it only applies when
allow_system_table_mods is enabled. The documentation already clearly
warns about the associated risks, so this should be fine.

thanks
Shveta