On Fri, Mar 21, 2025 at 12:50 PM Nisha Moond wrote:
> Thanks, Hou-san, for the review and fix patches. I’ve incorporated
> your suggestions.
> Attached are the v7 patches, including patch 002, which implements
> stats collection for 'multiple_unique_conflicts' in
> pg_stat_subscription_stats.
Thanks for updating the patches.
Here are some more comments:
1.
The comments atop of ReportApplyConflict() should be updated to reflect the
updates made to the input parameters.
2.
Add ConflictTupleInfo in typedefs.list.
3.
Few typos exiting => existing
4.
$node_subscriber->wait_for_log(
qr/ERROR: conflict detected on relation \"public.conf_tab\":
conflict=multiple_unique_conflicts/,
We should avoid adding the elevel('ERROR') body here as the format could be
changed depending on the log_error_verbosity. Please refer to d13ff82 for
details.
Please see attachment 0001 for the proposed changes.
5.
ok( $node_subscriber->log_contains(
qr/Key already exists in unique index
\"conf_tab_pkey\".*\n.*Key \(a\)=\(2\); existing local tuple \(2, 2, 2\);
remote tuple \(2, 3, 4\)./,
$log_offset),
'multiple_unique_conflicts detected during insertion for conf_tab_pkey
(a) = (2)'
);
...
ok( $node_subscriber->log_contains(
qr/Key already exists in unique index
\"conf_tab_b_key\".*\n.*Key \(b\)=\(3\); existing local tuple \(3, 3, 3\);
remote tuple \(2, 3, 4\)./,
$log_offset),
'multiple_unique_conflicts detected during insertion for conf_tab_b_key
(b) = (3)'
);
Currently, different detail lines are checked in separate test cases. It would
be clearer to merge these checks, ensuring comprehensive validation
including line breaks. See attachment 0002 for the proposed changes.
6.
The document related to the conflict log format should be updated. E.g., all the
places that mentioned insert|update_exists might need to mention the new
multi-key conflict as well. And it would be better to mention there would be
multiple detail lines in the new conflict. Please see attachment 0003 for the
proposed changes.
Best Regards,
Hou zj
From 20e2d08ba853a61523b6356b12cd600cc31217ad Mon Sep 17 00:00:00 2001
From: Hou Zhijie <houzj.f...@cn.fujitsu.com>
Date: Fri, 21 Mar 2025 13:50:27 +0800
Subject: [PATCH v2 1/3] fix comments and tests
---
src/backend/replication/logical/conflict.c | 9 ++-------
src/include/replication/conflict.h | 15 ++++++---------
src/test/subscription/t/035_conflicts.pl | 2 +-
src/tools/pgindent/typedefs.list | 1 +
4 files changed, 10 insertions(+), 17 deletions(-)
diff --git a/src/backend/replication/logical/conflict.c
b/src/backend/replication/logical/conflict.c
index b1614d3aaf6..9295c8ef6c1 100644
--- a/src/backend/replication/logical/conflict.c
+++ b/src/backend/replication/logical/conflict.c
@@ -91,15 +91,10 @@ GetTupleTransactionInfo(TupleTableSlot *localslot,
TransactionId *xmin,
* 'searchslot' should contain the tuple used to search the local tuple to be
* updated or deleted.
*
- * 'conflictslots' list contains the existing local tuples, if any, that
- * conflicts with the remote tuple. 'localxmins', 'localorigins', and 'localts'
- * provide the transaction information related to the existing local tuples.
- *
* 'remoteslot' should contain the remote new tuple, if any.
*
- * The 'conflictindexes' list represents the OIDs of the unique index that
- * triggered the constraint violation error. We use this to report the key
- * values for conflicting tuple.
+ * 'conflicttuples' should be a list composed of ConflictTupleInfo pointer.
+ * Refer to the ConflictTupleInfo structure comments for details.
*
* The caller must ensure that the index with the OID 'indexoid' is locked so
* that we can fetch and display the conflicting key value.
diff --git a/src/include/replication/conflict.h
b/src/include/replication/conflict.h
index 06d5d05c560..4d0bab0dd28 100644
--- a/src/include/replication/conflict.h
+++ b/src/include/replication/conflict.h
@@ -55,18 +55,15 @@ typedef enum
/*
- * Information for the exiting local tuple that caused the conflict.
+ * Information for the existing local tuple that caused the conflict.
*/
typedef struct ConflictTupleInfo
{
- TupleTableSlot *slot;
- Oid indexoid; /* conflicting index */
- TransactionId xmin; /* transaction ID that modified
the existing
- * local tuple
*/
- RepOriginId origin; /* which origin modified the
exiting local
- * tuple */
- TimestampTz ts; /* when the exiting local tuple
was modified
- * by the
origin */
+ TupleTableSlot *slot; /* tuple slot holding the conflicting local
tuple */
+ Oid indexoid; /* OID of the index where the conflict
occurred */
+ TransactionId xmin; /* transaction ID of the modification causing
the conflict */
+ RepOriginId origin; /* origin identifier of the modification */
+ TimestampTz ts; /* timestamp of when the modification on the
conflicting local tuple occurred */
} ConflictTupleInfo;
extern bool GetTupleTransactionInfo(TupleTableSlot *localslot,
diff --git a/src/test/subscription/t/035_conflicts.pl
b/src/test/subscription/t/035_conflicts.pl
index f1417e313db..37f8af07995 100644
--- a/src/test/subscription/t/035_conflicts.pl
+++ b/src/test/subscription/t/035_conflicts.pl
@@ -109,7 +109,7 @@ $node_publisher->safe_psql('postgres',
# Confirm that this causes an error on the subscriber
$node_subscriber->wait_for_log(
- qr/ERROR: conflict detected on relation \"public.conf_tab\":
conflict=multiple_unique_conflicts/,
+ qr/conflict detected on relation \"public.conf_tab\":
conflict=multiple_unique_conflicts/,
$log_offset);
ok( $node_subscriber->log_contains(
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index bfa276d2d35..3fbf5a4c212 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -480,6 +480,7 @@ ConditionVariableMinimallyPadded
ConditionalStack
ConfigData
ConfigVariable
+ConflictTupleInfo
ConflictType
ConnCacheEntry
ConnCacheKey
--
2.30.0.windows.2
From 12bd20f5d0deb1b31520a641df884034f8adf044 Mon Sep 17 00:00:00 2001
From: Hou Zhijie <houzj.f...@cn.fujitsu.com>
Date: Fri, 21 Mar 2025 14:09:01 +0800
Subject: [PATCH v2 2/3] merge tests
---
src/test/subscription/t/035_conflicts.pl | 52 ++++++++----------------
1 file changed, 16 insertions(+), 36 deletions(-)
diff --git a/src/test/subscription/t/035_conflicts.pl
b/src/test/subscription/t/035_conflicts.pl
index 37f8af07995..d06e64747cb 100644
--- a/src/test/subscription/t/035_conflicts.pl
+++ b/src/test/subscription/t/035_conflicts.pl
@@ -67,26 +67,16 @@ $node_publisher->safe_psql('postgres',
# Confirm that this causes an error on the subscriber
$node_subscriber->wait_for_log(
- qr/ERROR: conflict detected on relation \"public.conf_tab\":
conflict=multiple_unique_conflicts/,
+ qr/conflict detected on relation \"public.conf_tab\":
conflict=multiple_unique_conflicts.*
+.*Key already exists in unique index \"conf_tab_pkey\".*
+.*Key \(a\)=\(2\); existing local tuple \(2, 2, 2\); remote tuple \(2, 3, 4\).*
+.*Key already exists in unique index \"conf_tab_b_key\".*
+.*Key \(b\)=\(3\); existing local tuple \(3, 3, 3\); remote tuple \(2, 3, 4\).*
+.*Key already exists in unique index \"conf_tab_c_key\".*
+.*Key \(c\)=\(4\); existing local tuple \(4, 4, 4\); remote tuple \(2, 3,
4\)./,
$log_offset);
-ok( $node_subscriber->log_contains(
- qr/Key already exists in unique index
\"conf_tab_pkey\".*\n.*Key \(a\)=\(2\); existing local tuple \(2, 2, 2\);
remote tuple \(2, 3, 4\)./,
- $log_offset),
- 'multiple_unique_conflicts detected during insertion for conf_tab_pkey
(a) = (2)'
-);
-
-ok( $node_subscriber->log_contains(
- qr/Key already exists in unique index
\"conf_tab_b_key\".*\n.*Key \(b\)=\(3\); existing local tuple \(3, 3, 3\);
remote tuple \(2, 3, 4\)./,
- $log_offset),
- 'multiple_unique_conflicts detected during insertion for conf_tab_b_key
(b) = (3)'
-);
-
-ok( $node_subscriber->log_contains(
- qr/Key already exists in unique index
\"conf_tab_c_key\".*\n.*Key \(c\)=\(4\); existing local tuple \(4, 4, 4\);
remote tuple \(2, 3, 4\)./,
- $log_offset),
- 'multiple_unique_conflicts detected during insertion for conf_tab_c_key
(c) = (4)'
-);
+pass('multiple_unique_conflicts detected during update');
# Truncate table to get rid of the error
$node_subscriber->safe_psql('postgres', "TRUNCATE conf_tab;");
@@ -109,25 +99,15 @@ $node_publisher->safe_psql('postgres',
# Confirm that this causes an error on the subscriber
$node_subscriber->wait_for_log(
- qr/conflict detected on relation \"public.conf_tab\":
conflict=multiple_unique_conflicts/,
+ qr/conflict detected on relation \"public.conf_tab\":
conflict=multiple_unique_conflicts.*
+.*Key already exists in unique index \"conf_tab_pkey\".*
+.*Key \(a\)=\(6\); existing local tuple \(6, 6, 6\); remote tuple \(6, 7, 8\).*
+.*Key already exists in unique index \"conf_tab_b_key\".*
+.*Key \(b\)=\(7\); existing local tuple \(7, 7, 7\); remote tuple \(6, 7, 8\).*
+.*Key already exists in unique index \"conf_tab_c_key\".*
+.*Key \(c\)=\(8\); existing local tuple \(8, 8, 8\); remote tuple \(6, 7,
8\)./,
$log_offset);
-ok( $node_subscriber->log_contains(
- qr/Key already exists in unique index
\"conf_tab_pkey\".*\n.*Key \(a\)=\(6\); existing local tuple \(6, 6, 6\);
remote tuple \(6, 7, 8\)./,
- $log_offset),
- 'multiple_unique_conflicts detected during update for conf_tab_pkey (a)
= (6)'
-);
-
-ok( $node_subscriber->log_contains(
- qr/Key already exists in unique index
\"conf_tab_b_key\".*\n.*Key \(b\)=\(7\); existing local tuple \(7, 7, 7\);
remote tuple \(6, 7, 8\)./,
- $log_offset),
- 'multiple_unique_conflicts detected during update for conf_tab_b_key
(b) = (7)'
-);
-
-ok( $node_subscriber->log_contains(
- qr/Key already exists in unique index
\"conf_tab_c_key\".*\n.*Key \(c\)=\(8\); existing local tuple \(8, 8, 8\);
remote tuple \(6, 7, 8\)./,
- $log_offset),
- 'multiple_unique_conflicts detected during update for conf_tab_c_key
(c) = (8)'
-);
+pass('multiple_unique_conflicts detected during insert');
done_testing();
--
2.30.0.windows.2
From d4b7282365b7112953bcf3769f705bb4994ddd8d Mon Sep 17 00:00:00 2001
From: Hou Zhijie <houzj.f...@cn.fujitsu.com>
Date: Fri, 21 Mar 2025 14:27:23 +0800
Subject: [PATCH v2 3/3] add documents
---
doc/src/sgml/logical-replication.sgml | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/doc/src/sgml/logical-replication.sgml
b/doc/src/sgml/logical-replication.sgml
index 4637e898b9f..abade232473 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -1948,8 +1948,8 @@ DETAIL: <replaceable
class="parameter">detailed_explanation</replaceable>.
<para>
The <literal>Key</literal> section includes the key values of the
local
tuple that violated a unique constraint for
- <literal>insert_exists</literal> or <literal>update_exists</literal>
- conflicts.
+ <literal>insert_exists</literal>, <literal>update_exists</literal> or
+ <literal>multiple_unique_conflicts</literal> conflicts.
</para>
</listitem>
<listitem>
@@ -1958,8 +1958,8 @@ DETAIL: <replaceable
class="parameter">detailed_explanation</replaceable>.
tuple if its origin differs from the remote tuple for
<literal>update_origin_differs</literal> or
<literal>delete_origin_differs</literal>
conflicts, or if the key value conflicts with the remote tuple for
- <literal>insert_exists</literal> or <literal>update_exists</literal>
- conflicts.
+ <literal>insert_exists</literal>, <literal>update_exists</literal> or
+ <literal>multiple_unique_conflicts</literal> conflicts.
</para>
</listitem>
<listitem>
@@ -1995,6 +1995,16 @@ DETAIL: <replaceable
class="parameter">detailed_explanation</replaceable>.
The large column values are truncated to 64 bytes.
</para>
</listitem>
+ <listitem>
+ <para>
+ Note that in case of <literal>multiple_unique_conflicts</literal>
conflict,
+ multiple <replaceable
class="parameter">detailed_explanation</replaceable>
+ and <replaceable class="parameter">detail_values</replaceable> lines
+ will be generated, each detailing the conflict information associated
+ with distinct unique
+ constraints.
+ </para>
+ </listitem>
</itemizedlist>
</listitem>
</varlistentry>
--
2.30.0.windows.2
