From bf26cb39b608944b5e29094af34c032d522b2897 Mon Sep 17 00:00:00 2001 From: Vignesh C Date: Thu, 23 Apr 2026 17:48:09 +0530 Subject: [PATCH v2] 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. --- src/backend/replication/logical/conflict.c | 142 ++++++++++----------- 1 file changed, 69 insertions(+), 73 deletions(-) diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c index 48ef84ee924..b1482eb7617 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,15 @@ 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'); + if (first) + appendStringInfoString(buf, "tuple data not available (insufficient privileges)"); } /* @@ -258,6 +238,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 +253,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 +266,28 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo, if (err_msg->len == 0) { - appendStringInfoString(&err_detail, _("Could not apply remote change")); - - append_tuple_value_detail(&err_detail, - list_make2(remote_desc, search_desc), - true); + append_tuple_value_detail(&tuple_buf, + list_make2(remote_desc, search_desc)); + appendStringInfo(&err_detail, _("Could not apply remote change: %s.\n"), + tuple_buf.data); + 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), + tuple_buf.data); 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), + tuple_buf.data); /* * The origin that modified this row has been removed. This @@ -310,44 +297,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), + tuple_buf.data); } 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, + tuple_buf.data); 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), + tuple_buf.data); 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), + tuple_buf.data); /* 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), + tuple_buf.data); break; case CT_UPDATE_DELETED: - appendStringInfoString(&err_detail, _("Could not find the row to be updated")); - append_tuple_value_detail(&err_detail, - list_make2(remote_desc, search_desc), - true); + append_tuple_value_detail(&tuple_buf, + list_make2(remote_desc, search_desc)); + appendStringInfo(&err_detail, _("Could not find the row to be updated: %s.\n"), + tuple_buf.data); if (localts) { @@ -369,44 +359,50 @@ 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(&err_detail, - list_make2(remote_desc, search_desc), - false); + append_tuple_value_detail(&tuple_buf, + list_make2(remote_desc, search_desc)); + appendStringInfo(&err_detail, _("Could not find the row to be updated: %s."), + tuple_buf.data); 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), + tuple_buf.data); 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), + tuple_buf.data); /* 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), + tuple_buf.data); 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."), + tuple_buf.data); 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