Re: Conflict detection for update_deleted in logical replication

On Tue, Jan 7, 2025 at 6:04 PM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
>
> Attached the V19 patch which addressed comments in [1][2][3][4][5][6][7].
>

Here are a couple of initial review comments on v19 patch set:

1) The subscription option 'retain_conflict_info' remains set to
"true" for a subscription even after restarting the server with
'track_commit_timestamp=off', which can lead to incorrect behavior.
Steps to reproduce:
1. Start the server with 'track_commit_timestamp=ON'.
2. Create a subscription with (retain_conflict_info=ON).
3. Restart the server with 'track_commit_timestamp=OFF'.

- The apply worker starts successfully, and the subscription retains
'retain_conflict_info=true'. However, in this scenario, the
update_deleted conflict detection will not function correctly without
'track_commit_timestamp'.
```
postgres=# show track_commit_timestamp;
track_commit_timestamp
------------------------
off
(1 row)

postgres=# select subname, subretainconflictinfo from pg_subscription;
subname | subretainconflictinfo
---------+-----------------------
sub21 | t
sub22 | t
```

2) With the new parameter name change to "retain_conflict_info", the
error message for both the 'CREATE SUBSCRIPTION' and 'ALTER
SUBSCRIPTION' commands needs to be updated accordingly.

postgres=# create subscription sub11 connection 'dbname=postgres'
publication pub1 with (retain_conflict_info=on);
ERROR: detecting update_deleted conflicts requires
"track_commit_timestamp" to be enabled
postgres=# alter subscription sub12 set (retain_conflict_info=on);
ERROR: detecting update_deleted conflicts requires
"track_commit_timestamp" to be enabled

- Change the message to something similar - "retaining conflict info
requires "track_commit_timestamp" to be enabled".

--
Thanks,
Nisha