v35-0006-Review-comment-fixes-for-Implement-the-conflict-.patch
application/octet-stream
Filename: v35-0006-Review-comment-fixes-for-Implement-the-conflict-.patch
Type: application/octet-stream
Part: 5
From 3891223e662574347c461106bde2a1f1b18734ca Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Mon, 18 May 2026 11:25:52 +0000
Subject: [PATCH v35 6/9] Review comment fixes for Implement the conflict
insertion infrastructure for the conflict log table
Review comment fixes for Implement the conflict
insertion infrastructure for the conflict log table
---
src/backend/replication/logical/conflict.c | 59 ++++++++++------------
src/backend/replication/logical/worker.c | 2 +-
src/test/subscription/t/030_origin.pl | 4 +-
src/test/subscription/t/035_conflicts.pl | 4 +-
4 files changed, 32 insertions(+), 37 deletions(-)
diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c
index ed2ebae76a4..9764f2a5aaa 100644
--- a/src/backend/replication/logical/conflict.c
+++ b/src/backend/replication/logical/conflict.c
@@ -73,6 +73,17 @@ const ConflictLogColumnDef ConflictLogSchema[] = {
StaticAssertDecl(lengthof(ConflictLogSchema) == NUM_CONFLICT_ATTRS,
"ConflictLogSchema length mismatch");
+/* Schema for the elements within the 'local_conflicts' JSON array */
+static const ConflictLogColumnDef LocalConflictSchema[] =
+{
+ { .attname = "xid", .atttypid = XIDOID },
+ { .attname = "commit_ts", .atttypid = TIMESTAMPTZOID },
+ { .attname = "origin", .atttypid = TEXTOID },
+ { .attname = "key", .atttypid = JSONOID },
+ { .attname = "tuple", .atttypid = JSONOID }
+};
+
+#define NUM_LOCAL_CONFLICT_ATTRS lengthof(LocalConflictSchema)
static const char *const ConflictTypeNames[] = {
[CT_INSERT_EXISTS] = "insert_exists",
@@ -85,17 +96,7 @@ static const char *const ConflictTypeNames[] = {
[CT_MULTIPLE_UNIQUE_CONFLICTS] = "multiple_unique_conflicts"
};
-/* Schema for the elements within the 'local_conflicts' JSON array */
-static const ConflictLogColumnDef LocalConflictSchema[] =
-{
- { .attname = "xid", .atttypid = XIDOID },
- { .attname = "commit_ts", .atttypid = TIMESTAMPTZOID },
- { .attname = "origin", .atttypid = TEXTOID },
- { .attname = "key", .atttypid = JSONOID },
- { .attname = "tuple", .atttypid = JSONOID }
-};
-#define MAX_LOCAL_CONFLICT_INFO_ATTRS lengthof(LocalConflictSchema)
static int errcode_apply_conflict(ConflictType type);
static void errdetail_apply_conflict(EState *estate,
@@ -345,10 +346,8 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
*/
conflictlogrel = GetConflictLogDestAndTable(&dest);
- if (dest == CONFLICT_LOG_DEST_TABLE || dest == CONFLICT_LOG_DEST_ALL)
- log_dest_clt = true;
- if (dest == CONFLICT_LOG_DEST_LOG || dest == CONFLICT_LOG_DEST_ALL)
- log_dest_logfile = true;
+ log_dest_clt = CONFLICTS_LOGGED_TO_TABLE(dest);
+ log_dest_logfile = CONFLICTS_LOGGED_TO_FILE(dest);
/* Insert to table if requested. */
if (log_dest_clt)
@@ -380,9 +379,8 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
*/
ereport(elevel,
errcode_apply_conflict(type),
- errmsg("conflict detected on relation \"%s.%s\": conflict=%s",
- get_namespace_name(RelationGetNamespace(localrel)),
- RelationGetRelationName(localrel),
+ errmsg("conflict detected on relation \"%s\": conflict=%s",
+ RelationGetQualifiedRelationName(localrel),
ConflictTypeNames[type]),
errdetail("Conflict details are logged to the conflict log table: %s",
RelationGetRelationName(conflictlogrel)));
@@ -411,9 +409,8 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
/* Standard reporting with full internal details. */
ereport(elevel,
errcode_apply_conflict(type),
- errmsg("conflict detected on relation \"%s.%s\": conflict=%s",
- get_namespace_name(RelationGetNamespace(localrel)),
- RelationGetRelationName(localrel),
+ errmsg("conflict detected on relation \"%s\": conflict=%s",
+ RelationGetQualifiedRelationName(localrel),
ConflictTypeNames[type]),
errdetail_internal("%s", err_detail.data));
}
@@ -469,7 +466,7 @@ GetConflictLogDestAndTable(ConflictLogDest *log_dest)
*log_dest = GetLogDestination(MySubscription->conflictlogdest);
/* Quick exit if a conflict log table was not requested. */
- if (*log_dest == CONFLICT_LOG_DEST_LOG)
+ if (!CONFLICTS_LOGGED_TO_TABLE(*log_dest))
return NULL;
conflictlogrelid = MySubscription->conflictlogrelid;
@@ -489,13 +486,11 @@ GetConflictLogDestAndTable(ConflictLogDest *log_dest)
void
InsertConflictLogTuple(Relation conflictlogrel)
{
- int options = HEAP_INSERT_NO_LOGICAL;
-
/* A valid tuple must be prepared and stored in MyLogicalRepWorker. */
Assert(MyLogicalRepWorker->conflict_log_tuple != NULL);
heap_insert(conflictlogrel, MyLogicalRepWorker->conflict_log_tuple,
- GetCurrentCommandId(true), options, NULL);
+ GetCurrentCommandId(true), HEAP_INSERT_NO_LOGICAL, NULL);
/* Free conflict log tuple. */
heap_freetuple(MyLogicalRepWorker->conflict_log_tuple);
@@ -1077,9 +1072,9 @@ build_conflict_tupledesc(void)
{
TupleDesc tupdesc;
- tupdesc = CreateTemplateTupleDesc(MAX_LOCAL_CONFLICT_INFO_ATTRS);
+ tupdesc = CreateTemplateTupleDesc(NUM_LOCAL_CONFLICT_ATTRS);
- for (int i = 0; i < MAX_LOCAL_CONFLICT_INFO_ATTRS; i++)
+ for (int i = 0; i < NUM_LOCAL_CONFLICT_ATTRS; i++)
TupleDescInitEntry(tupdesc, (AttrNumber) (i + 1),
LocalConflictSchema[i].attname,
LocalConflictSchema[i].atttypid,
@@ -1120,8 +1115,8 @@ build_local_conflicts_json_array(EState *estate, Relation rel,
/* Process local conflict tuple list and prepare an array of JSON. */
foreach_ptr(ConflictTupleInfo, conflicttuple, conflicttuples)
{
- Datum values[MAX_LOCAL_CONFLICT_INFO_ATTRS] = {0};
- bool nulls[MAX_LOCAL_CONFLICT_INFO_ATTRS] = {0};
+ Datum values[NUM_LOCAL_CONFLICT_ATTRS] = {0};
+ bool nulls[NUM_LOCAL_CONFLICT_ATTRS] = {0};
char *origin_name = NULL;
HeapTuple tuple;
Datum json_datum;
@@ -1171,7 +1166,7 @@ build_local_conflicts_json_array(EState *estate, Relation rel,
else
nulls[attno] = true;
- Assert(attno + 1 == MAX_LOCAL_CONFLICT_INFO_ATTRS);
+ Assert(attno + 1 == NUM_LOCAL_CONFLICT_ATTRS);
tuple = heap_form_tuple(tupdesc, values, nulls);
@@ -1230,8 +1225,8 @@ prepare_conflict_log_tuple(EState *estate, Relation rel,
List *conflicttuples,
TupleTableSlot *remoteslot)
{
- Datum values[MAX_CONFLICT_ATTR_NUM] = {0};
- bool nulls[MAX_CONFLICT_ATTR_NUM] = {0};
+ Datum values[NUM_CONFLICT_ATTRS] = {0};
+ bool nulls[NUM_CONFLICT_ATTRS] = {0};
int attno;
char *remote_origin = NULL;
MemoryContext oldctx;
@@ -1297,7 +1292,7 @@ prepare_conflict_log_tuple(EState *estate, Relation rel,
conflict_type,
conflicttuples);
- Assert(attno + 1 == MAX_CONFLICT_ATTR_NUM);
+ Assert(attno + 1 == NUM_CONFLICT_ATTRS);
oldctx = MemoryContextSwitchTo(ApplyContext);
MyLogicalRepWorker->conflict_log_tuple =
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 469451c736a..ba74222f921 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -5688,7 +5688,7 @@ start_apply(XLogRecPtr origin_startpos)
/* Open conflict log table and insert the tuple. */
conflictlogrel = GetConflictLogDestAndTable(&dest);
- Assert(dest != CONFLICT_LOG_DEST_LOG);
+ Assert(CONFLICTS_LOGGED_TO_TABLE(dest));
InsertConflictLogTuple(conflictlogrel);
table_close(conflictlogrel, RowExclusiveLock);
diff --git a/src/test/subscription/t/030_origin.pl b/src/test/subscription/t/030_origin.pl
index 6bc6b7874c2..5f4d00bdd33 100644
--- a/src/test/subscription/t/030_origin.pl
+++ b/src/test/subscription/t/030_origin.pl
@@ -166,7 +166,7 @@ is($result, qq(32), 'The node_A data replicated to node_B');
$node_C->safe_psql('postgres', "UPDATE $tab SET a = 33 WHERE a = 32;");
$node_B->wait_for_log(
- qr/conflict detected on relation "public.$tab_unquoted": conflict=update_origin_differs.*\n.*DETAIL:.* Updating the row that was modified by a different origin ".*" in transaction [0-9]+ at .*: local row \(32\), remote row \(33\), replica identity \(a\)=\(32\)./
+ qr/conflict detected on relation "public.$tab": conflict=update_origin_differs.*\n.*DETAIL:.* Updating the row that was modified by a different origin ".*" in transaction [0-9]+ at .*: local row \(32\), remote row \(33\), replica identity \(a\)=\(32\)./
);
$node_B->safe_psql('postgres', "DELETE FROM $tab;");
@@ -182,7 +182,7 @@ is($result, qq(33), 'The node_A data replicated to node_B');
$node_C->safe_psql('postgres', "DELETE FROM $tab WHERE a = 33;");
$node_B->wait_for_log(
- qr/conflict detected on relation "public.$tab_unquoted": conflict=delete_origin_differs.*\n.*DETAIL:.* Deleting the row that was modified by a different origin ".*" in transaction [0-9]+ at .*: local row \(33\), replica identity \(a\)=\(33\).*/
+ qr/conflict detected on relation "public.$tab": conflict=delete_origin_differs.*\n.*DETAIL:.* Deleting the row that was modified by a different origin ".*" in transaction [0-9]+ at .*: local row \(33\), replica identity \(a\)=\(33\).*/
);
# The remaining tests no longer test conflict detection.
diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl
index 05c2179b9a8..4f3880e5b83 100644
--- a/src/test/subscription/t/035_conflicts.pl
+++ b/src/test/subscription/t/035_conflicts.pl
@@ -85,11 +85,11 @@ $node_subscriber->wait_for_log(
$log_offset);
# Verify the contents of the Conflict Log Table (CLT)
-# This section ensures that the clt contains the expected
+# This section ensures that the CLT contains the expected
# type and specific key data.
my $subid = $node_subscriber->safe_psql('postgres',
"SELECT oid FROM pg_subscription WHERE subname = 'sub_tab';");
-my $clt = "pg_conflict.pg_conflict_log_$subid";
+my $clt = "pg_conflict.pg_conflict_log_for_subid_$subid";
# Wait for the conflict to be logged in the CLT
my $log_check = $node_subscriber->poll_query_until(
--
2.53.0