Re: Proposal: Conflict log history table for Logical Replication

On Wed, Dec 3, 2025 at 3:27 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Wed, Dec 3, 2025 at 9:49 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > >
> > > relid | 16391
> > > schemaname | public
> > > relname | conf_tab
> > > conflict_type | multiple_unique_conflicts
> > > remote_xid | 761
> > > remote_commit_lsn | 0/01761400
> > > remote_commit_ts | 2025-12-02 15:02:07.045935+00
> > > remote_origin | pg_16406
> > > key_tuple |
> > > remote_tuple | {"a":2,"b":3,"c":4}
> > > local_conflicts |
> > > {"{\"xid\":\"773\",\"commit_ts\":\"2025-12-02T15:02:00.640253+00:00\",\"origin\":\"\",\"tuple\":{\"a\":2,\"b\":2,\"c\":2}}","{\"xid\":\"
> > > 773\",\"commit_ts\":\"2025-12-02T15:02:00.640253+00:00\",\"origin\":\"\",\"tuple\":{\"a\":3,\"b\":3,\"c\":3}}","{\"xid\":\"773\",\"commit_ts\":\"2025-12-02T
> > > 15:02:00.640253+00:00\",\"origin\":\"\",\"tuple\":{\"a\":4,\"b\":4,\"c\":4}}"}
> > >
> >
> > Thanks, it looks good. For the benefit of others, could you include a
> > brief note, perhaps in the commit message for now, describing how to
> > access or read this array column? We can remove it later.
>
> Thanks, okay, temporarily I have added in a commit message how we can
> fetch the data from the JSON array field. In next version I will add
> a test to get the conflict stored in conflict log history table and
> fetch from it.
>

I've reviewed the v9 patch and here are some comments:

The patch utilizes SPI for creating and dropping the conflict history
table, but I'm really not sure if it's okay because it's actually
affected by some GUC parameters such as default_tablespace and
default_toast_compression etc. Also, probably some hooks and event
triggers could be fired during the creation and removal. Is it
intentional behavior? I'm concerned that it would make investigation
harder if an issue happened in the user environment.

---
+ /* build and execute the CREATE TABLE query. */
+ appendStringInfo(&querybuf,
+ "CREATE TABLE %s.%s ("
+ "relid Oid,"
+ "schemaname TEXT,"
+ "relname TEXT,"
+ "conflict_type TEXT,"
+ "remote_xid xid,"
+ "remote_commit_lsn pg_lsn,"
+ "remote_commit_ts TIMESTAMPTZ,"
+ "remote_origin TEXT,"
+ "key_tuple JSON,"
+ "remote_tuple JSON,"
+ "local_conflicts JSON[])",
+ quote_identifier(get_namespace_name(namespaceId)),
+ quote_identifier(conflictrel));

If we want to use SPI for history table creation, we should use
qualified names in all the places including data types.

---
The patch doesn't create the dependency between the subscription and
the conflict history table. So users can entirely drop the schema
(with CASCADE option) where the history table is created. And once
dropping the schema along with the history table, ALTER SUBSCRIPTION
... SET (conflict_history_table = '') seems not to work (I got a
SEGV).

---
We can create the history table in pg_temp namespace but it should not
be allowed.

---
I think the conflict history table should not be transferred to the
new cluster when pg_upgrade since the table definition could be
different across major versions.

I got the following log when the publisher disables track_commit_timestamp:

local_conflicts |
{"{\"xid\":\"790\",\"commit_ts\":\"1999-12-31T16:00:00-08:00\",\"origin\":\"\",\"tuple\":{\"c\":1}}"}

I think we can omit commit_ts when it's omitted.

---
I think we should keep the history table name case-sensitive:

postgres(1:351685)=# create subscription sub connection
'dbname=postgres port=5551' publication pub with (conflict_log_table =
'LOGTABLE');
CREATE SUBSCRIPTION
postgres(1:351685)=# \d
List of relations
Schema | Name | Type | Owner
--------+----------+-------+----------
public | test | table | masahiko
public | logtable | table | masahiko
(2 rows)

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com