On Wed, Jul 31, 2024 at 7:40 AM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > > > > 2) > > apply_handle_delete_internal() > > > > --Do we need to check "(!edata->mtstate || edata->mtstate->operation != > > CMD_UPDATE)" in the else part as well? Can there be a scenario where during > > update flow, it is trying to delete from a partition and comes here, but > > till then > > that row is deleted already and we end up raising 'delete_missing' > > additionally > > instead of 'update_missing' > > alone? > > I think this shouldn't happen because the row to be deleted should have been > locked before entering the apply_handle_delete_internal(). Actually, calling > apply_handle_delete_internal() for cross-partition update is a big buggy > because the > row to be deleted has already been found in apply_handle_tuple_routing(), so > we > could have avoid scanning the tuple again. I have posted another patch to fix > this issue in thread[1].
Thanks for the details. > > Here is the V8 patch set. It includes the following changes: > Thanks for the patch. I verified that all the bugs reported so far are addressed. Few trivial comments: 1) 029_on_error.pl: --I did not understand the intent of this change. The existing insert would also have resulted in conflict (insert_exists) and we would have identified and skipped that. Why change to UPDATE? $node_publisher->safe_psql( 'postgres', qq[ BEGIN; -INSERT INTO tbl VALUES (1, NULL); +UPDATE tbl SET i = 2; PREPARE TRANSACTION 'gtx'; COMMIT PREPARED 'gtx'; ]); 2) logical-replication.sgml --In doc, shall we have 'delete_differ' first and then 'delete_missing', similar to what we have for update (first 'update_differ' and then 'update_missing') 3) logical-replication.sgml: "For instance, the origin in the above log indicates that the existing row was modified by a local change." --This clarification about origin was required when we had 'origin 0' in 'DETAILS'. Now we have "locally": "Key (c)=(1) already exists in unique index "t_pkey", which was modified locally in transaction 740". And thus shall we rephrase the concerned line ? thanks Shveta
