On Thu, Jul 2, 2026 at 1:39 PM Dilip Kumar <[email protected]> wrote: > > I rebased the remaining patches on top of HEAD. So far, I have run > pgindent and completed the doc merge for 0001. The 0002 and 0003 are > just rebased. >
Thanks, here are a few comments on the 001 patch: 1) conflict.h #include "access/xlogdefs.h" +#include "access/genam.h" #include "datatype/timestamp.h" We only need "Relation" here. Would it make sense to include "relcache.h" instead to keep the include dependency chain narrower? 2) worker_internal.h The comment above the new PARALLEL_TRANS_ERROR enum currently says: /* * The enum values must have the same order as the transaction state * transitions. */ PARALLEL_TRANS_ERROR does not strictly fit the transition order since it does not necessarily come after PARALLEL_TRANS_FINISHED. Suggestion: add something like below to avoid any confusion: "PARALLEL_TRANS_FINISHED and PARALLEL_TRANS_ERROR are mutually exclusive terminal states." 3) 035_conflicts.pl For the multiple_unique_conflicts tests, we are currently only verifying that one key exists out of the three, and we are also not checking whether all three conflicting rows are present in CLT. How about using a query like below to verify all three keys instead to provide stronger validation? "SELECT (unnest(local_conflicts)->'tuple'->>'a')::int FROM $clt WHERE conflict_type = 'multiple_unique_conflicts' ORDER BY 1;" Attached a 001 top-up patch with the above suggestions applied. Please consider them if you agree. -- Thanks, Nisha
From 329e970906b8970ed38cd1ac074dd0e4fb6ec25c Mon Sep 17 00:00:00 2001 From: Nisha Moond <[email protected]> Date: Fri, 3 Jul 2026 16:59:01 +0530 Subject: [PATCH v_nisha 2/2] nisha edits --- src/include/replication/conflict.h | 2 +- src/include/replication/worker_internal.h | 3 ++- src/test/subscription/t/035_conflicts.pl | 25 +++++++++++++---------- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/include/replication/conflict.h b/src/include/replication/conflict.h index 4db3a36d4df..06420ab0416 100644 --- a/src/include/replication/conflict.h +++ b/src/include/replication/conflict.h @@ -10,9 +10,9 @@ #define CONFLICT_H #include "access/xlogdefs.h" -#include "access/genam.h" #include "datatype/timestamp.h" #include "nodes/pg_list.h" +#include "utils/relcache.h" /* Avoid including execnodes.h here */ typedef struct EState EState; diff --git a/src/include/replication/worker_internal.h b/src/include/replication/worker_internal.h index 394f4c6265e..dc4d3db4abb 100644 --- a/src/include/replication/worker_internal.h +++ b/src/include/replication/worker_internal.h @@ -114,7 +114,8 @@ typedef struct LogicalRepWorker * State of the transaction in parallel apply worker. * * The enum values must have the same order as the transaction state - * transitions. + * transitions. PARALLEL_TRANS_FINISHED and PARALLEL_TRANS_ERROR are + * mutually exclusive terminal states. */ typedef enum ParallelTransState { diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl index 6a7d094712d..7e6d0707042 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -101,12 +101,13 @@ my $conflict_check = $node_subscriber->safe_psql('postgres', "SELECT count(*) >= 1 FROM $clt WHERE conflict_type = 'multiple_unique_conflicts';"); is($conflict_check, 't', 'Verified multiple_unique_conflicts logged into conflict log table'); -my $json_query = "SELECT local_conflicts FROM $clt;"; -my $raw_json = $node_subscriber->safe_psql('postgres', $json_query); - -# Verify that '2' is present inside the JSON structure using a regex -# This matches the key/value pattern for "a": 2 -like($raw_json, qr/\\"a\\":2/, 'Verified that key 2 exists in the local_conflicts'); +# Verify all 3 conflicting local rows are captured with correct key values +my $keys = $node_subscriber->safe_psql('postgres', + "SELECT (unnest(local_conflicts)->'tuple'->>'a')::int + FROM $clt WHERE conflict_type = 'multiple_unique_conflicts' + ORDER BY 1;"); +is($keys, "2\n3\n4", + 'local_conflicts captures correct conflicting key values for INSERT'); pass('multiple_unique_conflicts detected during insert'); @@ -153,11 +154,13 @@ $conflict_check = $node_subscriber->safe_psql('postgres', "SELECT count(*) >= 1 FROM $clt WHERE conflict_type = 'multiple_unique_conflicts';"); is($conflict_check, 't', 'Verified multiple_unique_conflicts logged into conflict log table'); -$raw_json = $node_subscriber->safe_psql('postgres', $json_query); - -# Verify that '6' is present inside the JSON structure using a regex -# This matches the key/value pattern for "a": 6 -like($raw_json, qr/\\"a\\":6/, 'Verified that key 6 exists in the local_conflicts'); +# Verify all 3 conflicting local rows are captured with correct key values +$keys = $node_subscriber->safe_psql('postgres', + "SELECT (unnest(local_conflicts)->'tuple'->>'a')::int + FROM $clt WHERE conflict_type = 'multiple_unique_conflicts' + ORDER BY 1;"); +is($keys, "6\n7\n8", + 'local_conflicts captures correct conflicting key values for UPDATE'); pass('multiple_unique_conflicts detected during update'); -- 2.50.1 (Apple Git-155)
