From 08932a7ae18cf7026e486f50d96596ec26cf0d90 Mon Sep 17 00:00:00 2001 From: Vignesh C 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 Author: vignesh C Reviewed-by: Amit Kapila Reviewed-by: Zhijie Hou Reviewed-by: Peter Smith 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 ": " 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