Re: Conflict detection and logging in logical replication

On Fri, Jul 26, 2024 at 3:03 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Thu, Jul 25, 2024 at 12:04 PM Zhijie Hou (Fujitsu)
> <houzj.fnst@fujitsu.com> wrote:
> > Here is the V6 patch set which addressed Shveta and Nisha's comments
> > in [1][2][3][4].
>
> Thanks for the patch.
> I tested the v6-0001 patch with partition table scenarios. Please
> review the following scenario where Pub updates a tuple, causing it to
> move from one partition to another on Sub.
>
> Setup:
> Pub:
>  create table tab (a int not null, b int not null);
>  alter table tab add constraint tab_pk primary key (a,b);
> Sub:
>  create table tab (a int not null, b int not null) partition by range (b);
>  alter table tab add constraint tab_pk primary key (a,b);
>  create table tab_1 partition of tab FOR values from (MINVALUE) TO (100);
>  create table tab_2 partition of tab FOR values from (101) TO (MAXVALUE);
>
> Test:
>  Pub: insert into tab values (1,1);
>  Sub: update tab set a=1 where a=1;  > just to make it Sub's origin
>  Sub: insert into tab values (1,101);
>  Pub: update b=101 where b=1; --> Both 'update_differ' and
> 'insert_exists' are detected.
>
> For non-partitioned tables, a similar update results in
> 'update_differ' and 'update_exists' conflicts. After detecting
> 'update_differ', the apply worker proceeds to apply the remote update
> and if a tuple with the updated key already exists, it raises
> 'update_exists'.
> This same behavior is expected for partitioned tables too.

Good catch. Yes, from the user's perspective, an update_* conflict
should be raised when performing an update operation. But internally
since we are deleting from one partition and inserting to another, we
are hitting insert_exist. To convert this insert_exist to udpate_exist
conflict, perhaps we need to change insert-operation to
update-operation as the default resolver is 'always apply update' in
case of update_differ. But not sure how much complexity it will add to
 the code. If it makes the code too complex, I think we can retain the
existing behaviour but document this multi-partition case. And in the
resolver patch, we can handle the resolution of insert_exists by
converting it to update. Thoughts?

thanks
Shveta