Re: Proposal: Conflict log history table for Logical Replication

From: shveta malik <shveta(dot)malik(at)gmail(dot)com>
To: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Dilip Kumar <DilipBalaut(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com>
Subject: Re: Proposal: Conflict log history table for Logical Replication
Date: 2026-06-15 05:57:34
Message-ID: CAJpy0uCoVPRvaAwgDz1EZww1PqHUd4B=S+F1Dh2F9kxKMpvJEw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

A few comments on v50:

1)
+ /*
+ * Conflict log tables are used internally for logical replication
+ * conflict logging and should not have extended statistics, as it
+ * could disrupt conflict logging.
+ */

Suggestion:
/*
* Conflict log tables are system-managed tables used internally for
* logical replication conflict logging. Similar to indexes, user-defined
* extended statistics are not supported on these tables at present.
*/

2)
Assertion hit in alter_sub_conflict_log_dest():

set allow_system_table_mods=on
create subscription sub1 connection '...' publication pub1
WITH(conflict_log_destination='log');
select oid from pg_subscription;

--CREATE clt USING SAME oid
create table pg_conflict.pg_conflict_log_16501(i int);

--change destination to table/all
alter subscription sub1 SET (CONFLICT_LOG_DESTINATION = 'ALL');

--this asserts here:
2026-06-15 11:10:40.540 IST [10626] LOG: background worker "logical
replication tablesync worker" (PID 11397) exited with exit code 1
TRAP: failed Assert("IsBinaryUpgrade"), File: "subscriptioncmds.c",
Line: 1554, PID: 10662

alter_sub_conflict_log_dest(): Assert(IsBinaryUpgrade);

Should we fix Assert or restrict table creation in pg_conflict scehma?
We allow it for toast-schema though when allow_system_table_mods=ON.
But I don't see a use case for allowing this.

3)
I'm not sure whether the comments (at [1]) regarding the TRY-CATCH
block were considered or perhaps overlooked. Please ignore this if you
are already taking them into account.

Doc-patch:
<caught my eye while checking something, not reviewing the doc-patch
rigth now though>

4)
+ direct user manipulation. Unlike <literal>pg_catalog</literal>, the
+ <literal>pg_catalog</literal> schema is not implicitly included in the
+ search path, so objects within it must be referenced explicitly or by
+ adjusting the search path.

Second pg_catalog should be replaced with pg_conflict

5)
+ as the <emphasis>conflict schema</emphasis>) contains system managed

+ The <literal>pg_conflict</literal> schema that contains system-managed

system managed or system-managed, we can use same at both the places.

~~

[1]: https://www.postgresql.org/message-id/CAJpy0uBSY7zTH%3D4TvAOS%3Dkj9vivBUc9NO%2BVp6KNw-Na9RiAsMg%40mail.gmail.com

thanks
Shveta

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2026-06-15 06:06:17 Re: Proposal: Conflict log history table for Logical Replication
Previous Message Kyotaro Horiguchi 2026-06-15 05:54:10 Re: Handle concurrent drop when doing whole database vacuum