From fbf83e3b1080d28b25ac64e4dc94cd4e33dd45bd Mon Sep 17 00:00:00 2001 From: Hayato Kuroda Date: Thu, 8 Jan 2026 23:26:39 +0900 Subject: [PATCH] Fix errdetail for logical replication conflict The apply worker describes details when it detects any conflicts. It contained a lot of information but did not follow our message style guidelines. This commit arranges these messages into complete sentences and translatable ones. Note that errdetail_apply_conflict() contains a huge number of if-else statements to avoid constructing statements at runtime. --- src/backend/replication/logical/conflict.c | 577 +++++++++++++++------ src/test/subscription/t/001_rep_changes.pl | 6 +- src/test/subscription/t/013_partition.pl | 14 +- src/test/subscription/t/029_on_error.pl | 2 +- src/test/subscription/t/030_origin.pl | 4 +- src/test/subscription/t/035_conflicts.pl | 56 +- 6 files changed, 467 insertions(+), 192 deletions(-) diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c index 93222ee3b88..270083698b6 100644 --- a/src/backend/replication/logical/conflict.c +++ b/src/backend/replication/logical/conflict.c @@ -44,17 +44,18 @@ static void errdetail_apply_conflict(EState *estate, Oid indexoid, TransactionId localxmin, RepOriginId localorigin, TimestampTz localts, StringInfo err_msg); -static char *build_tuple_value_details(EState *estate, ResultRelInfo *relinfo, - ConflictType type, - TupleTableSlot *searchslot, - TupleTableSlot *localslot, - TupleTableSlot *remoteslot, - Oid indexoid); +static void build_tuple_value_details(EState *estate, ResultRelInfo *relinfo, + ConflictType type, + char **key_desc, + TupleTableSlot *searchslot, char **search_desc, + TupleTableSlot *localslot, char **local_desc, + TupleTableSlot *remoteslot, char **remote_desc, + Oid indexoid); static char *build_index_value_desc(EState *estate, Relation localrel, TupleTableSlot *slot, Oid indexoid); /* - * Get the xmin and commit timestamp data (origin and timestamp) associated + * Get the xmin and commit timestamp data (origin and timestamp) associatedq * with the provided local row. * * Return true if the commit timestamp data was found, false otherwise. @@ -124,10 +125,10 @@ ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel, ereport(elevel, errcode_apply_conflict(type), - errmsg("conflict detected on relation \"%s.%s\": conflict=%s", + errmsg("conflict %s detected on relation \"%s.%s\"", + ConflictTypeNames[type], get_namespace_name(RelationGetNamespace(localrel)), - RelationGetRelationName(localrel), - ConflictTypeNames[type]), + RelationGetRelationName(localrel)), errdetail_internal("%s", err_detail.data)); } @@ -188,13 +189,6 @@ errcode_apply_conflict(ConflictType type) /* * Add an errdetail() line showing conflict detail. - * - * The DETAIL line comprises of two parts: - * 1. Explanation of the conflict type, including the origin and commit - * timestamp of the existing local row. - * 2. Display of conflicting key, existing local row, remote new row, and - * replica identity columns, if any. The remote old row is excluded as its - * information is covered in the replica identity columns. */ static void errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo, @@ -205,8 +199,17 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo, StringInfo err_msg) { StringInfoData err_detail; - char *val_desc; char *origin_name; + char *key_desc = NULL; + char *search_desc = NULL; + char *local_desc = NULL; + char *remote_desc = NULL; + + build_tuple_value_details(estate, relinfo, type, &key_desc, + searchslot, &search_desc, + localslot, &local_desc, + remoteslot, &remote_desc, + indexoid); initStringInfo(&err_detail); @@ -219,105 +222,421 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo, Assert(OidIsValid(indexoid) && CheckRelationOidLockedByMe(indexoid, RowExclusiveLock, true)); - if (localts) + if (err_msg->len == 0 && remote_desc) { - if (localorigin == InvalidRepOriginId) - appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified locally in transaction %u at %s."), - get_rel_name(indexoid), - localxmin, timestamptz_to_str(localts)); - 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."), - get_rel_name(indexoid), origin_name, - localxmin, timestamptz_to_str(localts)); - - /* - * The origin that modified this row has been removed. This - * can happen if the origin was created by a different apply - * worker and its associated subscription and origin were - * dropped after updating the row, or if the origin was - * manually dropped by the user. - */ + if (search_desc) + appendStringInfo(&err_detail, + _("Could not apply remote row %s by using %s.\n"), + remote_desc, search_desc); else - appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified by a non-existent origin in transaction %u at %s."), - get_rel_name(indexoid), - localxmin, timestamptz_to_str(localts)); + appendStringInfo(&err_detail, + _("Could not apply remote row %s.\n"), + remote_desc); + } + + if (key_desc && local_desc) + { + if (localts) + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Key %s already exists in unique index \"%s\", modified locally in transaction %u at %s: local row %s."), + key_desc, get_rel_name(indexoid), + localxmin, + timestamptz_to_str(localts), + local_desc); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Key %s already exists in unique index \"%s\", modified by origin \"%s\" in transaction %u at %s: local row %s."), + key_desc, get_rel_name(indexoid), + origin_name, localxmin, + timestamptz_to_str(localts), + local_desc); + + /* + * The origin that modified this row has been removed. This + * can happen if the origin was created by a different + * apply worker and its associated subscription and origin + * were dropped after updating the row, or if the origin + * was manually dropped by the user. + */ + else + appendStringInfo(&err_detail, + _("Key %s already exists in unique index \"%s\", modified by a non-existent origin in transaction %u at %s: local row %s."), + key_desc, get_rel_name(indexoid), + localxmin, + timestamptz_to_str(localts), + local_desc); + } + else + appendStringInfo(&err_detail, + _("Key %s already exists in unique index \"%s\", modified in transaction %u: local row %s."), + key_desc, get_rel_name(indexoid), + localxmin, local_desc); + } + else if (!key_desc && local_desc) + { + if (localts) + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Unique index \"%s\" rejects applying due to local row %s, modified locally in transaction %u at %s."), + get_rel_name(indexoid), local_desc, + localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Unique index \"%s\" rejects applying due to local row %s, modified by origin \"%s\" in transaction %u at %s."), + get_rel_name(indexoid), local_desc, + origin_name, localxmin, + timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Unique index \"%s\" rejects applying due to local row %s, modified by a non-existent origin in transaction %u at %s."), + get_rel_name(indexoid), local_desc, + localxmin, + timestamptz_to_str(localts)); + } + else + appendStringInfo(&err_detail, + _("Unique index \"%s\" rejects applying due to local row %s, modified in transaction %u."), + get_rel_name(indexoid), local_desc, + localxmin); + } + else if (key_desc && !local_desc) + { + if (localts) + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Key %s already exists in unique index \"%s\", modified locally in transaction %u at %s."), + key_desc, get_rel_name(indexoid), localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Key %s already exists in unique index \"%s\", modified by origin \"%s\" in transaction %u at %s."), + key_desc, get_rel_name(indexoid), origin_name, localxmin, + timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Key %s already exists in unique index \"%s\", modified by a non-existent origin in transaction %u at %s."), + key_desc, get_rel_name(indexoid), + localxmin, + timestamptz_to_str(localts)); + } + else + appendStringInfo(&err_detail, + _("Key %s already exists in unique index \"%s\", modified in transaction %u."), + key_desc, get_rel_name(indexoid), + localxmin); } else - appendStringInfo(&err_detail, _("Key already exists in unique index \"%s\", modified in transaction %u."), - get_rel_name(indexoid), localxmin); + { + if (localts) + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Remote row violates unique constraint \"%s\", modified locally in transaction %u at %s."), + get_rel_name(indexoid), localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Remote row violates unique constraint \"%s\", modified by origin \"%s\" in transaction %u at %s."), + get_rel_name(indexoid), origin_name, + localxmin, + timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Remote row violates unique constraint \"%s\", modified by a non-existent origin in transaction %u at %s."), + get_rel_name(indexoid), localxmin, + timestamptz_to_str(localts)); + } + else + appendStringInfo(&err_detail, + _("Remote row violates unique constraint \"%s\", modified in transaction %u."), + get_rel_name(indexoid), localxmin); + } break; case CT_UPDATE_ORIGIN_DIFFERS: - if (localorigin == InvalidRepOriginId) - appendStringInfo(&err_detail, _("Updating the row that was modified locally in transaction %u at %s."), - localxmin, timestamptz_to_str(localts)); - 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)); - - /* The origin that modified this row has been removed. */ + if (remote_desc) + appendStringInfo(&err_detail, + _("Remote row %s was applied but previously modified by different origin.\n"), + remote_desc); + + if (search_desc && local_desc) + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Local row %s detected by %s is being updated, but it was previously modified locally in transaction %u at %s."), + local_desc, search_desc, localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Local row %s detected by %s is being updated, but it was previously modified by origin \"%s\" in transaction %u at %s."), + local_desc, search_desc, origin_name, + localxmin, timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Local row %s detected by %s is being updated, but it was previously modified by a non-existent origin in transaction %u at %s."), + local_desc, search_desc, localxmin, + timestamptz_to_str(localts)); + } + else if (!search_desc && local_desc) + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Local row %s is being updated, but it was previously modified locally in transaction %u at %s."), + local_desc, localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Local row %s is being updated, but it was previously modified by origin \"%s\" in transaction %u at %s."), + local_desc, origin_name, localxmin, + timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Local row %s is being updated, but it was previously modified by a non-existent origin in transaction %u at %s."), + local_desc, localxmin, + timestamptz_to_str(localts)); + } + else if (search_desc && !local_desc) + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Local row detected by %s is being updated, but it was previously modified locally in transaction %u at %s."), + search_desc, localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Local row detected by %s is being updated, but it was previously modified by origin \"%s\" in transaction %u at %s."), + search_desc, origin_name, localxmin, + timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Local row detected by %s is being updated, but it was previously modified by a non-existent origin in transaction %u at %s."), + search_desc, localxmin, + timestamptz_to_str(localts)); + } 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)); + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Local row is being updated, but it was previously modified locally in transaction %u at %s."), + localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Local row is being updated, but it was previously modified by origin \"%s\" in transaction %u at %s."), + origin_name, localxmin, + timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Local row is being updated, but it was previously modified by a non-existent origin in transaction %u at %s."), + localxmin, + timestamptz_to_str(localts)); + } break; case CT_UPDATE_DELETED: - if (localts) + if (remote_desc) + { + if (search_desc) + appendStringInfo(&err_detail, + _("Could not find remote row %s by using %s.\n"), + remote_desc, search_desc); + else + appendStringInfo(&err_detail, + _("Could not find remote row %s.\n"), + remote_desc); + } + + if (local_desc) { if (localorigin == InvalidRepOriginId) - appendStringInfo(&err_detail, _("The row to be updated was deleted locally in transaction %u at %s."), - localxmin, timestamptz_to_str(localts)); + appendStringInfo(&err_detail, + _("Local row %s was previously deleted locally in transaction %u at %s."), + local_desc, localxmin, + timestamptz_to_str(localts)); else if (replorigin_by_oid(localorigin, true, &origin_name)) - appendStringInfo(&err_detail, _("The row to be updated was deleted by a different origin \"%s\" in transaction %u at %s."), - origin_name, localxmin, timestamptz_to_str(localts)); + appendStringInfo(&err_detail, + _("Local row %s was previously deleted by a different origin \"%s\" in transaction %u at %s."), + local_desc, + origin_name, localxmin, + timestamptz_to_str(localts)); /* The origin that modified this row has been removed. */ else - appendStringInfo(&err_detail, _("The row to be updated was deleted by a non-existent origin in transaction %u at %s."), - localxmin, timestamptz_to_str(localts)); + appendStringInfo(&err_detail, + _("Local row %s was previously deleted by a non-existent origin in transaction %u at %s."), + local_desc, localxmin, + timestamptz_to_str(localts)); + } else - appendStringInfo(&err_detail, _("The row to be updated was deleted.")); + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Local row was previously deleted locally in transaction %u at %s."), + localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Local row was previously deleted by a different origin \"%s\" in transaction %u at %s."), + origin_name, localxmin, + timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Local row was previously deleted by a non-existent origin in transaction %u at %s."), + localxmin, + timestamptz_to_str(localts)); + } break; case CT_UPDATE_MISSING: - appendStringInfoString(&err_detail, _("Could not find the row to be updated.")); + if (search_desc && remote_desc) + appendStringInfo(&err_detail, + _("Could not find remote row %s by using %s."), + remote_desc, search_desc); + else if (!search_desc && remote_desc) + appendStringInfo(&err_detail, + _("Could not find remote row %s."), + remote_desc); + else if (search_desc && !remote_desc) + appendStringInfo(&err_detail, + _("Could not find the row to be updated by using %s."), + search_desc); + else + appendStringInfo(&err_detail, + _("Could not find the row to be updated.")); + break; - case CT_DELETE_ORIGIN_DIFFERS: - if (localorigin == InvalidRepOriginId) - appendStringInfo(&err_detail, _("Deleting the row that was modified locally in transaction %u at %s."), - localxmin, timestamptz_to_str(localts)); - 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)); - - /* The origin that modified this row has been removed. */ + case CT_DELETE_MISSING: + if (search_desc) + appendStringInfo(&err_detail, + _("Could not find the row to be deleted by using %s."), + search_desc); 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)); + appendStringInfo(&err_detail, + _("Could not find the row to be deleted.")); break; - case CT_DELETE_MISSING: - appendStringInfoString(&err_detail, _("Could not find the row to be deleted.")); + case CT_DELETE_ORIGIN_DIFFERS: + if (search_desc && local_desc) + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Local row %s detected by %s is being deleted, but it was previously modified locally in transaction %u at %s."), + local_desc, search_desc, localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Local row %s detected by %s is being deleted, but it was previously modified by origin \"%s\" in transaction %u at %s."), + local_desc, search_desc, origin_name, + localxmin, timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Local row %s detected by %s is being deleted, but it was previously modified by a non-existent origin in transaction %u at %s."), + local_desc, search_desc, localxmin, + timestamptz_to_str(localts)); + } + else if (!search_desc && local_desc) + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Local row %s is being deleted, but it was previously modified locally in transaction %u at %s."), + local_desc, localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Local row %s is being deleted, but it was previously modified by origin \"%s\" in transaction %u at %s."), + local_desc, origin_name, localxmin, + timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Local row %s is being deleted, but it was previously modified by a non-existent origin in transaction %u at %s."), + local_desc, localxmin, + timestamptz_to_str(localts)); + } + else if (search_desc && !local_desc) + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Local row detected by %s is being deleted, but it was previously modified locally in transaction %u at %s."), + search_desc, localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Local row detected by %s is being deleted, but it was previously modified by origin \"%s\" in transaction %u at %s."), + search_desc, origin_name, localxmin, + timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Local row detected by %s is being deleted, but it was previously modified by a non-existent origin in transaction %u at %s."), + search_desc, localxmin, + timestamptz_to_str(localts)); + } + else + { + if (localorigin == InvalidRepOriginId) + appendStringInfo(&err_detail, + _("Local row is being deleted, but it was previously modified locally in transaction %u at %s."), + localxmin, + timestamptz_to_str(localts)); + else if (replorigin_by_oid(localorigin, true, &origin_name)) + appendStringInfo(&err_detail, + _("Local row is being deleted, but it was previously modified by origin \"%s\" in transaction %u at %s."), + origin_name, localxmin, + timestamptz_to_str(localts)); + + /* The origin that modified this row has been removed. */ + else + appendStringInfo(&err_detail, + _("Local row is being deleted, but it was previously modified by a non-existent origin in transaction %u at %s."), + localxmin, + timestamptz_to_str(localts)); + } + break; } - Assert(err_detail.len > 0); - - val_desc = build_tuple_value_details(estate, relinfo, type, searchslot, - localslot, remoteslot, indexoid); + if (key_desc) + pfree(key_desc); + if (search_desc) + pfree(search_desc); + if (local_desc) + pfree(local_desc); + if (remote_desc) + pfree(remote_desc); - /* - * Next, append the key values, existing local row, remote row, and - * replica identity columns after the message. - */ - if (val_desc) - appendStringInfo(&err_detail, "\n%s", val_desc); + Assert(err_detail.len > 0); /* * Insert a blank line to visually separate the new detail line from the @@ -332,27 +651,26 @@ errdetail_apply_conflict(EState *estate, ResultRelInfo *relinfo, /* * Helper function to build the additional details for conflicting key, * existing local row, remote row, and replica identity columns. + * Results are set at xxx_desc. * - * If the return value is NULL, it indicates that the current user lacks - * permissions to view the columns involved. + * If the output is NULL, it indicates that the current user lacks permissions + * to view the columns involved. */ -static char * +static void build_tuple_value_details(EState *estate, ResultRelInfo *relinfo, ConflictType type, - TupleTableSlot *searchslot, - TupleTableSlot *localslot, - TupleTableSlot *remoteslot, + char **key_desc, + TupleTableSlot *searchslot, char **search_desc, + TupleTableSlot *localslot, char **local_desc, + TupleTableSlot *remoteslot, char **remote_desc, Oid indexoid) { Relation localrel = relinfo->ri_RelationDesc; Oid relid = RelationGetRelid(localrel); TupleDesc tupdesc = RelationGetDescr(localrel); - StringInfoData tuple_value; - char *desc = NULL; - - Assert(searchslot || localslot || remoteslot); - initStringInfo(&tuple_value); + Assert((searchslot && search_desc) || (localslot && local_desc) || + (remoteslot && remote_desc)); /* * Report the conflicting key values in the case of a unique constraint @@ -363,10 +681,8 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo, { Assert(OidIsValid(indexoid) && localslot); - desc = build_index_value_desc(estate, localrel, localslot, indexoid); - - if (desc) - appendStringInfo(&tuple_value, _("Key %s"), desc); + *key_desc = build_index_value_desc(estate, localrel, localslot, + indexoid); } if (localslot) @@ -375,23 +691,8 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo, * The 'modifiedCols' only applies to the new tuple, hence we pass * NULL for the existing local row. */ - desc = ExecBuildSlotValueDescription(relid, localslot, tupdesc, - NULL, 64); - - if (desc) - { - if (tuple_value.len > 0) - { - appendStringInfoString(&tuple_value, "; "); - appendStringInfo(&tuple_value, _("existing local row %s"), - desc); - } - else - { - appendStringInfo(&tuple_value, _("Existing local row %s"), - desc); - } - } + *local_desc = ExecBuildSlotValueDescription(relid, localslot, tupdesc, + NULL, 64); } if (remoteslot) @@ -407,21 +708,9 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo, */ modifiedCols = bms_union(ExecGetInsertedCols(relinfo, estate), ExecGetUpdatedCols(relinfo, estate)); - desc = ExecBuildSlotValueDescription(relid, remoteslot, tupdesc, - modifiedCols, 64); - - if (desc) - { - if (tuple_value.len > 0) - { - appendStringInfoString(&tuple_value, "; "); - appendStringInfo(&tuple_value, _("remote row %s"), desc); - } - else - { - appendStringInfo(&tuple_value, _("Remote row %s"), desc); - } - } + *remote_desc = ExecBuildSlotValueDescription(relid, remoteslot, + tupdesc, modifiedCols, + 64); } if (searchslot) @@ -434,6 +723,7 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo, * cases, thus such indexes are not used here. */ Oid replica_index = GetRelationIdentityOrPK(localrel); + char *desc = NULL; Assert(type != CT_INSERT_EXISTS); @@ -449,27 +739,18 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo, if (desc) { - if (tuple_value.len > 0) - { - appendStringInfoString(&tuple_value, "; "); - appendStringInfo(&tuple_value, OidIsValid(replica_index) - ? _("replica identity %s") - : _("replica identity full %s"), desc); - } - else - { - appendStringInfo(&tuple_value, OidIsValid(replica_index) - ? _("Replica identity %s") - : _("Replica identity full %s"), desc); - } - } - } + StringInfoData ri_desc; - if (tuple_value.len == 0) - return NULL; + initStringInfo(&ri_desc); + appendStringInfo(&ri_desc, OidIsValid(replica_index) + ? _("replica identity %s") + : _("replica identity full %s"), desc); + + *search_desc = ri_desc.data; - appendStringInfoChar(&tuple_value, '.'); - return tuple_value.data; + pfree(desc); + } + } } /* diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl index 58e4b2398ff..e04fa3420b9 100644 --- a/src/test/subscription/t/001_rep_changes.pl +++ b/src/test/subscription/t/001_rep_changes.pl @@ -366,15 +366,15 @@ $node_publisher->wait_for_catchup('tap_sub'); my $logfile = slurp_file($node_subscriber->logfile, $log_location); like( $logfile, - qr/conflict detected on relation "public.tab_full_pk": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(1, quux\); replica identity \(a\)=\(1\)/m, + qr/conflict update_missing detected on relation "public.tab_full_pk".*\n.*DETAIL:.* Could not find remote row \(1, quux\) by using replica identity \(a\)=\(1\)./m, 'update target row is missing'); like( $logfile, - qr/conflict detected on relation "public.tab_full": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(26\); replica identity full \(25\)/m, + qr/conflict update_missing detected on relation "public.tab_full".*\n.*DETAIL:.* Could not find remote row \(26\) by using replica identity full \(25\)./m, 'update target row is missing'); like( $logfile, - qr/conflict detected on relation "public.tab_full_pk": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(2\)/m, + qr/conflict delete_missing detected on relation "public.tab_full_pk".*\n.*DETAIL:.* Could not find the row to be deleted by using replica identity \(a\)=\(2\)./m, 'delete target row is missing'); $node_subscriber->append_conf('postgresql.conf', diff --git a/src/test/subscription/t/013_partition.pl b/src/test/subscription/t/013_partition.pl index 4f90bc9a62a..5540f8d7103 100644 --- a/src/test/subscription/t/013_partition.pl +++ b/src/test/subscription/t/013_partition.pl @@ -369,19 +369,19 @@ $node_publisher->wait_for_catchup('sub2'); my $logfile = slurp_file($node_subscriber1->logfile(), $log_location); like( $logfile, - qr/conflict detected on relation "public.tab1_2_2": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(null, 4, quux\); replica identity \(a\)=\(4\)/, + qr/conflict update_missing detected on relation "public.tab1_2_2".*\n.*DETAIL:.* Could not find remote row \(null, 4, quux\) by using replica identity \(a\)=\(4\)/, 'update target row is missing in tab1_2_2'); like( $logfile, - qr/conflict detected on relation "public.tab1_1": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(1\)/, + qr/conflict delete_missing detected on relation "public.tab1_1".*\n.*DETAIL:.* Could not find the row to be deleted by using replica identity \(a\)=\(1\)/, 'delete target row is missing in tab1_1'); like( $logfile, - qr/conflict detected on relation "public.tab1_2_2": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(4\)/, + qr/conflict delete_missing detected on relation "public.tab1_2_2".*\n.*DETAIL:.* Could not find the row to be deleted by using replica identity \(a\)=\(4\)/, 'delete target row is missing in tab1_2_2'); like( $logfile, - qr/conflict detected on relation "public.tab1_def": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(10\)/, + qr/conflict delete_missing detected on relation "public.tab1_def".*\n.*DETAIL:.* Could not find the row to be deleted by using replica identity \(a\)=\(10\)/, 'delete target row is missing in tab1_def'); # Tests for replication using root table identity and schema @@ -786,11 +786,11 @@ $node_publisher->wait_for_catchup('sub2'); $logfile = slurp_file($node_subscriber1->logfile(), $log_location); like( $logfile, - qr/conflict detected on relation "public.tab2_1": conflict=update_missing.*\n.*DETAIL:.* Could not find the row to be updated.*\n.*Remote row \(pub_tab2, quux, 5\); replica identity \(a\)=\(5\)/, + qr/conflict update_missing detected on relation "public.tab2_1".*\n.*DETAIL:.* Could not find remote row \(pub_tab2, quux, 5\) by using replica identity \(a\)=\(5\)/, 'update target row is missing in tab2_1'); like( $logfile, - qr/conflict detected on relation "public.tab2_1": conflict=delete_missing.*\n.*DETAIL:.* Could not find the row to be deleted.*\n.*Replica identity \(a\)=\(1\)/, + qr/conflict delete_missing detected on relation "public.tab2_1".*\n.*DETAIL:.* Could not find the row to be deleted by using replica identity \(a\)=\(1\)/, 'delete target row is missing in tab2_1'); # Enable the track_commit_timestamp to detect the conflict when attempting @@ -809,7 +809,7 @@ $node_publisher->wait_for_catchup('sub_viaroot'); $logfile = slurp_file($node_subscriber1->logfile(), $log_location); like( $logfile, - qr/conflict detected on relation "public.tab2_1": conflict=update_origin_differs.*\n.*DETAIL:.* Updating the row that was modified locally in transaction [0-9]+ at .*\n.*Existing local row \(yyy, null, 3\); remote row \(pub_tab2, quux, 3\); replica identity \(a\)=\(3\)/, + qr/conflict update_origin_differs detected on relation "public.tab2_1".*\n.*DETAIL:.* Remote row \(pub_tab2, quux, 3\) was applied but previously modified by different origin. *\n.*Local row \(yyy, null, 3\) detected by replica identity \(a\)=\(3\) is being updated, but it was previously modified locally in transaction [0-9]+ at .*/, 'updating a row that was modified by a different origin'); # The remaining tests no longer test conflict detection. diff --git a/src/test/subscription/t/029_on_error.pl b/src/test/subscription/t/029_on_error.pl index 79271be684d..f87062dacc8 100644 --- a/src/test/subscription/t/029_on_error.pl +++ b/src/test/subscription/t/029_on_error.pl @@ -30,7 +30,7 @@ sub test_skip_lsn # ERROR with its CONTEXT when retrieving this information. my $contents = slurp_file($node_subscriber->logfile, $offset); $contents =~ - qr/conflict detected on relation "public.tbl".*\n.*DETAIL:.* Key already exists in unique index "tbl_pkey", modified by .*origin.* transaction \d+ at .*\n.*Key \(i\)=\(\d+\); existing local row .*; remote row .*\n.*CONTEXT:.* for replication target relation "public.tbl" in transaction \d+, finished at ([[:xdigit:]]+\/[[:xdigit:]]+)/m + qr/conflict .* detected on relation "public.tbl".*\n.*DETAIL:.* Could not apply remote row .*\n.*Key \(i\)=\(\d+\) already exists in unique index "tbl_pkey", modified by .*origin.* in transaction \d+ at .*: local row .*\n.*CONTEXT:.* for replication target relation "public.tbl" in transaction \d+, finished at ([[:xdigit:]]+\/[[:xdigit:]]+)/m or die "could not get error-LSN"; my $lsn = $1; diff --git a/src/test/subscription/t/030_origin.pl b/src/test/subscription/t/030_origin.pl index f2ab30f5809..68c51d49285 100644 --- a/src/test/subscription/t/030_origin.pl +++ b/src/test/subscription/t/030_origin.pl @@ -163,7 +163,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": conflict=update_origin_differs.*\n.*DETAIL:.* Updating the row that was modified by a different origin ".*" in transaction [0-9]+ at .*\n.*Existing local row \(32\); remote row \(33\); replica identity \(a\)=\(32\)/ + qr/conflict update_origin_differs detected on relation "public.tab".*\n.*DETAIL:.* Remote row \(33\) was applied but previously modified by different origin.*\n.*Local row \(32\) detected by replica identity \(a\)=\(32\) is being updated, but it was previously modified by origin ".*" in transaction [0-9]+ at .*/ ); $node_B->safe_psql('postgres', "DELETE FROM tab;"); @@ -179,7 +179,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": conflict=delete_origin_differs.*\n.*DETAIL:.* Deleting the row that was modified by a different origin ".*" in transaction [0-9]+ at .*\n.*Existing local row \(33\); replica identity \(a\)=\(33\)/ + qr/conflict delete_origin_differs detected on relation "public.tab".*\n.*DETAIL:.* Local row \(33\) detected by replica identity \(a\)=\(33\) is being deleted, but it was previously modified by origin ".*" in transaction [0-9]+ at .*/ ); # 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 ddc75e23fb0..d7e72ee5d8e 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -77,13 +77,11 @@ $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.* -.*Key already exists in unique index \"conf_tab_pkey\".* -.*Key \(a\)=\(2\); existing local row \(2, 2, 2\); remote row \(2, 3, 4\).* -.*Key already exists in unique index \"conf_tab_b_key\".* -.*Key \(b\)=\(3\); existing local row \(3, 3, 3\); remote row \(2, 3, 4\).* -.*Key already exists in unique index \"conf_tab_c_key\".* -.*Key \(c\)=\(4\); existing local row \(4, 4, 4\); remote row \(2, 3, 4\)./, + qr/conflict multiple_unique_conflicts detected on relation \"public.conf_tab\".* +.*Could not apply remote row \(2, 3, 4\).* +.*Key \(a\)=\(2\) already exists in unique index \"conf_tab_pkey\", modified in transaction .*: local row \(2, 2, 2\).* +.*Key \(b\)=\(3\) already exists in unique index \"conf_tab_b_key\", modified in transaction .*: local row \(3, 3, 3\).* +.*Key \(c\)=\(4\) already exists in unique index \"conf_tab_c_key\", modified in transaction .*: local row \(4, 4, 4\)./, $log_offset); pass('multiple_unique_conflicts detected during insert'); @@ -109,13 +107,11 @@ $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.* -.*Key already exists in unique index \"conf_tab_pkey\".* -.*Key \(a\)=\(6\); existing local row \(6, 6, 6\); remote row \(6, 7, 8\).* -.*Key already exists in unique index \"conf_tab_b_key\".* -.*Key \(b\)=\(7\); existing local row \(7, 7, 7\); remote row \(6, 7, 8\).* -.*Key already exists in unique index \"conf_tab_c_key\".* -.*Key \(c\)=\(8\); existing local row \(8, 8, 8\); remote row \(6, 7, 8\)./, + qr/conflict multiple_unique_conflicts detected on relation \"public.conf_tab\".* +.*Could not apply remote row \(6, 7, 8\).* +.*Key \(a\)=\(6\) already exists in unique index \"conf_tab_pkey\", modified in transaction .*: local row \(6, 6, 6\).* +.*Key \(b\)=\(7\) already exists in unique index \"conf_tab_b_key\", modified in transaction .*: local row \(7, 7, 7\).* +.*Key \(c\)=\(8\) already exists in unique index \"conf_tab_c_key\", modified in transaction .*: local row \(8, 8, 8\)./, $log_offset); pass('multiple_unique_conflicts detected during update'); @@ -137,11 +133,10 @@ $node_publisher->safe_psql('postgres', "INSERT INTO conf_tab_2 VALUES (55,2,3);"); $node_subscriber->wait_for_log( - qr/conflict detected on relation \"public.conf_tab_2_p1\": conflict=multiple_unique_conflicts.* -.*Key already exists in unique index \"conf_tab_2_p1_pkey\".* -.*Key \(a\)=\(55\); existing local row \(55, 2, 3\); remote row \(55, 2, 3\).* -.*Key already exists in unique index \"conf_tab_2_p1_a_b_key\".* -.*Key \(a, b\)=\(55, 2\); existing local row \(55, 2, 3\); remote row \(55, 2, 3\)./, + qr/conflict multiple_unique_conflicts detected on relation \"public.conf_tab_2_p1\".* +.*Could not apply remote row \(55, 2, 3\).* +.*Key \(a\)=\(55\) already exists in unique index \"conf_tab_2_p1_pkey\", modified in transaction .*: local row \(55, 2, 3\).* +.*Key \(a, b\)=\(55, 2\) already exists in unique index \"conf_tab_2_p1_a_b_key\", modified in transaction .*: local row \(55, 2, 3\)./, $log_offset); pass('multiple_unique_conflicts detected on a leaf partition during insert'); @@ -318,9 +313,8 @@ $node_A->wait_for_catchup($subname_BA); my $logfile = slurp_file($node_B->logfile(), $log_location); like( $logfile, - qr/conflict detected on relation "public.tab": conflict=delete_origin_differs.* -.*DETAIL:.* Deleting the row that was modified locally in transaction [0-9]+ at .* -.*Existing local row \(1, 3\); replica identity \(a\)=\(1\)/, + qr/conflict delete_origin_differs detected on relation "public.tab".* +.*DETAIL:.* Local row \(1, 3\) detected by replica identity \(a\)=\(1\) is being deleted, but it was previously modified locally in transaction [0-9]+ at .*/, 'delete target row was modified in tab'); $log_location = -s $node_A->logfile; @@ -332,9 +326,9 @@ $node_B->wait_for_catchup($subname_AB); $logfile = slurp_file($node_A->logfile(), $log_location); like( $logfile, - qr/conflict detected on relation "public.tab": conflict=update_deleted.* -.*DETAIL:.* The row to be updated was deleted locally in transaction [0-9]+ at .* -.*Remote row \(1, 3\); replica identity \(a\)=\(1\)/, + qr/conflict update_deleted detected on relation "public.tab".* +.*DETAIL:.* Could not find remote row \(1, 3\) by using replica identity \(a\)=\(1\).* +.*Local row was previously deleted locally in transaction [0-9]+ at .*./, 'update target row was deleted in tab'); # Remember the next transaction ID to be assigned @@ -380,9 +374,9 @@ $node_B->wait_for_catchup($subname_AB); $logfile = slurp_file($node_A->logfile(), $log_location); like( $logfile, - qr/conflict detected on relation "public.tab": conflict=update_deleted.* -.*DETAIL:.* The row to be updated was deleted locally in transaction [0-9]+ at .* -.*Remote row \(2, 4\); replica identity full \(2, 2\)/, + qr/conflict update_deleted detected on relation "public.tab".* +.*DETAIL:.* Could not find remote row \(2, 4\) by using replica identity full \(2, 2\).* +.*Local row was previously deleted locally in transaction [0-9]+ at .*./, 'update target row was deleted in tab'); ############################################################################### @@ -539,9 +533,9 @@ if ($injection_points_supported != 0) $logfile = slurp_file($node_A->logfile(), $log_location); like( $logfile, - qr/conflict detected on relation "public.tab": conflict=update_deleted.* -.*DETAIL:.* The row to be updated was deleted locally in transaction [0-9]+ at .* -.*Remote row \(1, 2\); replica identity full \(1, 1\)/, + qr/conflict update_deleted detected on relation "public.tab".* +.*DETAIL:.* Could not find remote row \(1, 2\) by using replica identity full \(1, 1\).* +.*Local row was previously deleted locally in transaction [0-9]+ at .*./, 'update target row was deleted in tab'); # Remember the next transaction ID to be assigned -- 2.47.3