Re: [PATCH] Disallow column lists on tables with REPLICA IDENTITY FULL when publishing updates/deletes

From: Mohit Kumar <mohit(dot)bash(at)gmail(dot)com>
To: Zhijie Hou <houzhijie22(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Disallow column lists on tables with REPLICA IDENTITY FULL when publishing updates/deletes
Date: 2026-10-25 13:32:38
Message-ID: CAGQj2CpyM53YoK+Y3b8bEHStWzz8W4ahWrSD=7KWUHo=10e4-Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Hou,

Thank you for the quick review and for pointing me to the earlier
discussion on Bug #19434.

Reading through that thread, the trade-offs between DDL-time prevention and
DML-time
enforcement make complete sense. The main challenge with DDL-time checks is
that
the table or publication state can evolve through other DDL operations
after creation:

1. ALTER PUBLICATION ... SET (publish = ...):
The current patch already handles this: if a publication contains a
table with a
column list and REPLICA IDENTITY FULL, altering 'publish' to add
update/delete is
rejected in AlterPublicationOptions().

2. ALTER TABLE ... REPLICA IDENTITY FULL:
If a table is already part of a publication with a column list and
update/delete
enabled, changing the replica identity to FULL would re-introduce the
invalid state.
We could guard ATExecReplicaIdentity() to reject this, or rely on the
existing
runtime check in CheckCmdReplicaIdentity() and document this in the
comments.

3. Attaching partitions with REPLICA IDENTITY FULL to a published
partitioned table.

My motivation came from the fact that CREATE PUBLICATION ... (cols) on an
*existing*
REPLICA IDENTITY FULL table is an immediate, guaranteed failure for any
subsequent
updates/deletes. Even if we cannot close every secondary DDL edge case
without adding
complexity across tablecmds.c, catching it upfront in
CheckPubRelationColumnList()
prevents the most common migration trap.

Would you recommend:
(a) Keeping the patch focused on publication DDL (CREATE/ALTER PUBLICATION)
with clear
comments explaining why subsequent ALTER TABLE actions fall back to runtime
checks?
OR
(b) Also extending the check to ATExecReplicaIdentity() in tablecmds.c?

I would be happy to update the patch and comments based on what the
community prefers.

Best Regards,
Mohit

On Sun, Oct 25, 2026 at 3:41 PM Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:

> Hi,
>
> On Sun, Oct 25, 2026 at 4:41 AM Mohit Kumar <mohit(dot)bash(at)gmail(dot)com> wrote:
> > I would like to propose a patch to catch an invalid publication
> configuration
> > at DDL time rather than deferring the failure to runtime DML.
> >
> > Problem:
> > Currently, CREATE PUBLICATION and ALTER PUBLICATION allow specifying
> a column
> > list on tables that have REPLICA IDENTITY FULL, even when the
> publication publishes
> > UPDATE and DELETE operations (the default).
> >
> > For example:
> >
> > CREATE TABLE mytab (id int, val text, extra text);
> > ALTER TABLE mytab REPLICA IDENTITY FULL;
> >
> > -- 1. This succeeds silently with no warning or error:
> > CREATE PUBLICATION mypub FOR TABLE mytab (id, val);
> >
> > -- 2. Inserts work fine:
> > INSERT INTO mytab VALUES (1, 'hello', 'world');
> >
> > -- 3. Any subsequent UPDATE or DELETE fails in production:
> > UPDATE mytab SET val = 'updated' WHERE id = 1;
> > -- ERROR: cannot update table "mytab"
> > -- DETAIL: Column list used by the publication does not cover the
> replica identity.
> >
> > This recently caused an unexpected production outage for us: the
> schema migration
> > succeeded cleanly without error in CI/CD, but application web
> traffic immediately
> > began failing on subsequent UPDATE and DELETE operations.
> >
> > Proposed Solution:
> > Because REPLICA IDENTITY FULL requires the entire row to identify
> tuples on the
> > subscriber, a column list on such a table can never work for updates
> or deletes.
>
> Thank you for the report and for sharing the patch.
>
> Could you take a look at a related proposal thread from earlier[1]? It
> explains
> why this check wasn't included initially, and it also highlights some
> corner
> cases that might need attention if similar checks were added.
>
> I'm not opposed to adding checks in the DDL path, but I suspect the
> proposal
> here may run into the same considerations as the earlier one. It might be
> worth
> confirming that once, so these cases can be handled correctly from the
> start.
> And if the patch decides to leave some corner cases aside, a brief note in
> the
> comments explaining why would be very helpful for future readers.
>
> [1]
> https://www.postgresql.org/message-id/flat/CAKz%3D%3Db%2B-%2BQFiqtpzDYXYfCUjoKJUN9VjpdfM-Fou44C9KaQCZg%40mail.gmail.com#35881adb054b6f2de1b97ee1a179a497
>
> Best Regards,
> Zhijie Hou
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Burd 2026-10-25 14:28:26 Re: ZSTD TOAST compression, and an extensible compression method encoding
Previous Message Ishjaap Singh II 2026-10-25 12:35:24 Re: psql: avoid over-reading unterminated prompt escapes