On Thu, Jul 18, 2024 at 7:52 AM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > Attach the V5 patch set which changed the following.
Thanks for the patch. Tested that previous reported issues are fixed. Please have a look at below scenario, I was expecting it to raise 'update_differ' but it raised both 'update_differ' and 'delete_differ' together: ------------------------- Pub: create table tab (a int not null, b int primary key); create publication pub1 for table tab; Sub (partitioned table): create table tab (a int not null, b int primary key) partition by range (b); create table tab_1 partition of tab for values from (minvalue) to (100); create table tab_2 partition of tab for values from (100) to (maxvalue); create subscription sub1 connection '.......' publication pub1 WITH (detect_conflict=true); Pub - insert into tab values (1,1); Sub - update tab set b=1000 where a=1; Pub - update tab set b=1000 where a=1; -->update_missing detected correctly as b=1 will not be found on sub. Pub - update tab set b=1 where b=1000; -->update_differ expected, but it gives both update_differ and delete_differ. ------------------------- Few trivial comments: 1) Commit msg: For insert_exists conflict, the log can include origin and commit timestamp details of the conflicting key with track_commit_timestamp enabled. --Please add update_exists as well. 2) execReplication.c: Return false if there is no conflict and *conflictslot is set to NULL. --This gives a feeling that this function will return false if both the conditions are true. But instead first one is the condition, while the second is action. Better to rephrase to: Returns false if there is no conflict. Sets *conflictslot to NULL in such a case. Or Sets *conflictslot to NULL and returns false in case of no conflict. 3) FindConflictTuple() shares some code parts with RelationFindReplTupleByIndex() and RelationFindReplTupleSeq() for checking status in 'res'. Is it worth making a function to be used in all three. thanks Shveta
