v3-0001-replication-fix-translation-issues-in-tuple-value.patch

From 08932a7ae18cf7026e486f50d96596ec26cf0d90 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Thu, 23 Apr 2026 17:48:09 +0530
Subject: [PATCH v3] replication: fix translation issues in tuple value detail
 messages

append_tuple_value_detail() constructed user-visible messages using
separately translated fragments such as ": ", ", ", and ".",. This
makes correct translation difficult or impossible in some languages.

Refactor append_tuple_value_detail() to move all punctuation and
sentence construction to the callers, which now use a single
translatable string with a %s placeholder for the tuple data.

Reported-by: David Rowley <dgrowleyml@gmail.com>
Author: vignesh C <vignesh21@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Zhijie Hou <houzj.fnst@fujitsu.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/227279.1775956328%40sss.pgh.pa.us#8f3a5f50543556c60cc5a13270cb7ba4
---
 src/backend/replication/logical/conflict.c | 147 +++++++++++----------
 1 file changed, 76 insertions(+), 71 deletions(-)

diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c
index 48ef84ee924..b3d2a608650 100644
--- a/src/backend/replication/logical/conflict.c
+++ b/src/backend/replication/logical/conflict.c
@@ -192,8 +192,7 @@ errcode_apply_conflict(ConflictType type)
  * local row, remote row, and replica identity columns.
  */
 static void
-append_tuple_value_detail(StringInfo buf, List *tuple_values,
-						  bool need_newline)
+append_tuple_value_detail(StringInfo buf, List *tuple_values)
 {
 	bool		first = true;
 
@@ -209,34 +208,21 @@ append_tuple_value_detail(StringInfo buf, List *tuple_values,
 		if (!tuple_value)
 			continue;
 
-		if (first)
-		{
-			/*
-			 * translator: The colon is used as a separator in conflict
-			 * messages. The first part, built in the caller, describes what
-			 * happened locally; the second part lists the conflicting keys
-			 * and tuple data.
-			 */
-			appendStringInfoString(buf, _(": "));
-		}
-		else
-		{
-			/*
-			 * translator: This is a separator in a list of conflicting keys
-			 * and tuple data.
-			 */
-			appendStringInfoString(buf, _(", "));
-		}
+		if (!first)
+			appendStringInfoString(buf, ", ");
 
 		appendStringInfoString(buf, tuple_value);
 		first = false;
 	}
+}
 
-	/* translator: This is the terminator of a conflict message */
-	appendStringInfoString(buf, _("."));
-
-	if (need_newline)
-		appendStringInfoChar(buf, '\n');
+/*
+ * Return ": <tuple>" if tuple_buf has data, otherwise return an empty string.
+ */
+static inline const char *
+optional_tuple_suffix(const StringInfo tuple_buf)
+{
+	return (tuple_buf->len > 0) ? psprintf(": %s", tuple_buf->data) : "";
 }
 
 /*
@@ -258,6 +244,7 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo,
 						 StringInfo err_msg)
 {
 	StringInfoData err_detail;
+	StringInfoData tuple_buf;
 	char	   *origin_name;
 	char	   *key_desc = NULL;
 	char	   *local_desc = NULL;
@@ -272,6 +259,7 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo,
 				   indexoid);
 
 	initStringInfo(&err_detail);
+	initStringInfo(&tuple_buf);
 
 	/* Construct a detailed message describing the type of conflict */
 	switch (type)
@@ -284,23 +272,30 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo,
 
 			if (err_msg->len == 0)
 			{
-				appendStringInfoString(&err_detail, _("Could not apply remote change"));
+				append_tuple_value_detail(&tuple_buf,
+										  list_make2(remote_desc, search_desc));
 
-				append_tuple_value_detail(&err_detail,
-										  list_make2(remote_desc, search_desc),
-										  true);
+				appendStringInfo(&err_detail, _("Could not apply remote change%s.\n"),
+								 optional_tuple_suffix(&tuple_buf));
+
+				resetStringInfo(&tuple_buf);
 			}
 
+			append_tuple_value_detail(&tuple_buf,
+									  list_make2(key_desc, local_desc));
+
 			if (localts)
 			{
 				if (localorigin == InvalidReplOriginId)
-					appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified locally in transaction %u at %s"),
+					appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified locally in transaction %u at %s%s."),
 									 get_rel_name(indexoid),
-									 localxmin, timestamptz_to_str(localts));
+									 localxmin, timestamptz_to_str(localts),
+									 optional_tuple_suffix(&tuple_buf));
 				else if (replorigin_by_oid(localorigin, true, &origin_name))
-					appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified by origin \"%s\" in transaction %u at %s"),
+					appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified by origin \"%s\" in transaction %u at %s%s."),
 									 get_rel_name(indexoid), origin_name,
-									 localxmin, timestamptz_to_str(localts));
+									 localxmin, timestamptz_to_str(localts),
+									 optional_tuple_suffix(&tuple_buf));
 
 				/*
 				 * The origin that modified this row has been removed. This
@@ -310,44 +305,47 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo,
 				 * manually dropped by the user.
 				 */
 				else
-					appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified by a non-existent origin in transaction %u at %s"),
+					appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified by a non-existent origin in transaction %u at %s%s."),
 									 get_rel_name(indexoid),
-									 localxmin, timestamptz_to_str(localts));
+									 localxmin, timestamptz_to_str(localts),
+									 optional_tuple_suffix(&tuple_buf));
 			}
 			else
-				appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified in transaction %u"),
-								 get_rel_name(indexoid), localxmin);
-
-			append_tuple_value_detail(&err_detail,
-									  list_make2(key_desc, local_desc), false);
+				appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified in transaction %u%s."),
+								 get_rel_name(indexoid), localxmin,
+								 optional_tuple_suffix(&tuple_buf));
 
 			break;
 
 		case CT_UPDATE_ORIGIN_DIFFERS:
+			append_tuple_value_detail(&tuple_buf,
+									  list_make3(local_desc, remote_desc,
+												 search_desc));
+
 			if (localorigin == InvalidReplOriginId)
-				appendStringInfo(&err_detail, _("Updating the row that was modified locally in transaction %u at %s"),
-								 localxmin, timestamptz_to_str(localts));
+				appendStringInfo(&err_detail, _("Updating the row that was modified locally in transaction %u at %s%s."),
+								 localxmin, timestamptz_to_str(localts),
+								 optional_tuple_suffix(&tuple_buf));
 			else if (replorigin_by_oid(localorigin, true, &origin_name))
-				appendStringInfo(&err_detail, _("Updating the row that was modified by a different origin \"%s\" in transaction %u at %s"),
-								 origin_name, localxmin, timestamptz_to_str(localts));
+				appendStringInfo(&err_detail, _("Updating the row that was modified by a different origin \"%s\" in transaction %u at %s%s."),
+								 origin_name, localxmin,
+								 timestamptz_to_str(localts),
+								 optional_tuple_suffix(&tuple_buf));
 
 			/* The origin that modified this row has been removed. */
 			else
-				appendStringInfo(&err_detail, _("Updating the row that was modified by a non-existent origin in transaction %u at %s"),
-								 localxmin, timestamptz_to_str(localts));
-
-			append_tuple_value_detail(&err_detail,
-									  list_make3(local_desc, remote_desc,
-												 search_desc), false);
+				appendStringInfo(&err_detail, _("Updating the row that was modified by a non-existent origin in transaction %u at %s%s."),
+								 localxmin, timestamptz_to_str(localts),
+								 optional_tuple_suffix(&tuple_buf));
 
 			break;
 
 		case CT_UPDATE_DELETED:
-			appendStringInfoString(&err_detail, _("Could not find the row to be updated"));
+			append_tuple_value_detail(&tuple_buf,
+									  list_make2(remote_desc, search_desc));
 
-			append_tuple_value_detail(&err_detail,
-									  list_make2(remote_desc, search_desc),
-									  true);
+			appendStringInfo(&err_detail, _("Could not find the row to be updated%s.\n"),
+							 optional_tuple_suffix(&tuple_buf));
 
 			if (localts)
 			{
@@ -369,44 +367,51 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo,
 			break;
 
 		case CT_UPDATE_MISSING:
-			appendStringInfoString(&err_detail, _("Could not find the row to be updated"));
+			append_tuple_value_detail(&tuple_buf,
+									  list_make2(remote_desc, search_desc));
 
-			append_tuple_value_detail(&err_detail,
-									  list_make2(remote_desc, search_desc),
-									  false);
+			appendStringInfo(&err_detail, _("Could not find the row to be updated%s."),
+							 optional_tuple_suffix(&tuple_buf));
 
 			break;
 
 		case CT_DELETE_ORIGIN_DIFFERS:
+			append_tuple_value_detail(&tuple_buf,
+									  list_make3(local_desc, remote_desc,
+												 search_desc));
+
 			if (localorigin == InvalidReplOriginId)
-				appendStringInfo(&err_detail, _("Deleting the row that was modified locally in transaction %u at %s"),
-								 localxmin, timestamptz_to_str(localts));
+				appendStringInfo(&err_detail, _("Deleting the row that was modified locally in transaction %u at %s%s."),
+								 localxmin, timestamptz_to_str(localts),
+								 optional_tuple_suffix(&tuple_buf));
 			else if (replorigin_by_oid(localorigin, true, &origin_name))
-				appendStringInfo(&err_detail, _("Deleting the row that was modified by a different origin \"%s\" in transaction %u at %s"),
-								 origin_name, localxmin, timestamptz_to_str(localts));
+				appendStringInfo(&err_detail, _("Deleting the row that was modified by a different origin \"%s\" in transaction %u at %s%s."),
+								 origin_name, localxmin,
+								 timestamptz_to_str(localts),
+								 optional_tuple_suffix(&tuple_buf));
 
 			/* The origin that modified this row has been removed. */
 			else
-				appendStringInfo(&err_detail, _("Deleting the row that was modified by a non-existent origin in transaction %u at %s"),
-								 localxmin, timestamptz_to_str(localts));
-
-			append_tuple_value_detail(&err_detail,
-									  list_make3(local_desc, remote_desc,
-												 search_desc), false);
+				appendStringInfo(&err_detail, _("Deleting the row that was modified by a non-existent origin in transaction %u at %s%s."),
+								 localxmin, timestamptz_to_str(localts),
+								 optional_tuple_suffix(&tuple_buf));
 
 			break;
 
 		case CT_DELETE_MISSING:
-			appendStringInfoString(&err_detail, _("Could not find the row to be deleted"));
+			append_tuple_value_detail(&tuple_buf,
+									  list_make1(search_desc));
 
-			append_tuple_value_detail(&err_detail,
-									  list_make1(search_desc), false);
+			appendStringInfo(&err_detail, _("Could not find the row to be deleted%s."),
+							 optional_tuple_suffix(&tuple_buf));
 
 			break;
 	}
 
 	Assert(err_detail.len > 0);
 
+	pfree(tuple_buf.data);
+
 	/*
 	 * Insert a blank line to visually separate the new detail line from the
 	 * existing ones.
-- 
2.43.0