Simplify code building the LR conflict messages
While reviewing another conflict-related thread, I noticed that the
existing conflict messages are currently getting built using
unexpected logic.
e.g
----------
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);
}
----------
I couldn't think of a reason why the "; " string needed to be
separated from the rest of the message like that. And when you combine
the strings, the logic easily collapses into a single statement with
less code and greater readability.
There were several code fragments like this. Here is a patch to modify them.
Thoughts?
======
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachments:
v1-0001-Simplify-the-code-building-the-conflict-messages.patchapplication/octet-stream; name=v1-0001-Simplify-the-code-building-the-conflict-messages.patchDownload
From f5bdcbc593ae7925c505a2907cc2ea9dd1750532 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Fri, 28 Nov 2025 12:23:59 +1100
Subject: [PATCH v1] Simplify the code building the conflict messages
---
src/backend/replication/logical/conflict.c | 39 ++++++------------------------
1 file changed, 8 insertions(+), 31 deletions(-)
diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c
index 1669559..9d726c5 100644
--- a/src/backend/replication/logical/conflict.c
+++ b/src/backend/replication/logical/conflict.c
@@ -379,19 +379,9 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo,
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);
- }
- }
+ appendStringInfo(&tuple_value, tuple_value.len > 0
+ ? _("; existing local row %s")
+ : _("Existing local row %s"), desc);
}
if (remoteslot)
@@ -411,17 +401,9 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo,
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);
- }
- }
+ appendStringInfo(&tuple_value, tuple_value.len > 0
+ ? _("; remote row %s")
+ : _("Remote row %s"), desc);
}
if (searchslot)
@@ -450,18 +432,13 @@ 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);
- }
+ ? _("; replica identity %s")
+ : _("; replica identity full %s"), desc);
else
- {
appendStringInfo(&tuple_value, OidIsValid(replica_index)
? _("Replica identity %s")
: _("Replica identity full %s"), desc);
- }
}
}
--
1.8.3.1
Peter Smith <smithpb2250@gmail.com> writes:
I couldn't think of a reason why the "; " string needed to be
separated from the rest of the message like that. And when you combine
the strings, the logic easily collapses into a single statement with
less code and greater readability.
... and, probably, less ability of the compiler to verify that the
variadic arguments match the format string. I think you've taken
this a bit too far.
regards, tom lane
On Fri, Nov 28, 2025 at 1:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Smith <smithpb2250@gmail.com> writes:
I couldn't think of a reason why the "; " string needed to be
separated from the rest of the message like that. And when you combine
the strings, the logic easily collapses into a single statement with
less code and greater readability.... and, probably, less ability of the compiler to verify that the
variadic arguments match the format string. I think you've taken
this a bit too far.
Hi Tom,
Thank you for the feedback. Could you please clarify which aspect is
of concern so I can address it properly:
* Is the concern about having a StringInfo format string that starts
with a semicolon? If so, I noticed there are already multiple similar
examples in the codebase (see: `grep -r . -e 'StringInfo(.*";.*%'`),
so I understood this pattern to be acceptable.
* Or is it the use of the ternary operator to select the format
string? If that's the issue, please note that my patch only introduced
the ternary operator for the first two code fragments. The third
fragment already uses ternaries in the same way on master, so I
understood that to be an established pattern as well.
I'd like to make sure I understand your concern correctly so I can
revise the patch appropriately.
======
Kind Regards,
Peter Smith
Fujitsu Australia
Peter Smith <smithpb2250@gmail.com> writes:
On Fri, Nov 28, 2025 at 1:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
... and, probably, less ability of the compiler to verify that the
variadic arguments match the format string. I think you've taken
this a bit too far.
* Or is it the use of the ternary operator to select the format
string? If that's the issue, please note that my patch only introduced
the ternary operator for the first two code fragments. The third
fragment already uses ternaries in the same way on master, so I
understood that to be an established pattern as well.
Sadly, you were copying bad code that we need to fix.
I'd like to make sure I understand your concern correctly so I can
revise the patch appropriately.
My concern is that we follow a coding style that the compiler can
check. Most C compilers can only verify that printf-like format
strings match the actual arguments if the format string is a constant.
So if I fat-finger the format string in this example:
--- a/src/backend/replication/logical/conflict.c
+++ b/src/backend/replication/logical/conflict.c
@@ -383,7 +383,7 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo,
if (tuple_value.len > 0)
{
appendStringInfoString(&tuple_value, "; ");
- appendStringInfo(&tuple_value, _("existing local row %s"),
+ appendStringInfo(&tuple_value, _("existing local row %d"),
desc);
}I'll hear about it:
conflict.c: In function 'build_tuple_value_details':
conflict.c:386:38: warning: format '%d' expects argument of type 'int', but argument 3 has type 'char *' [-Wformat=]
appendStringInfo(&tuple_value, _("existing local row %d"),
^~~~~~~~~~~~~~~~~~~~~~~
conflict.c:386:36: note: in expansion of macro '_'
appendStringInfo(&tuple_value, _("existing local row %d"),
^
But I don't trust the compiler to see through a ternary expression
and check (both!) format strings against the actuals.
In a quick test, the gcc version I have handy does seem to be able to
do that, but I don't think we should rely on that. Format strings
are something that is way too easy to get wrong, and most of us just
expect the compiler to cross-check them for us, so coding patterns
that might silently disable such compiler warnings are best avoided.
(There's also some magic going on here to allow the compiler to see
through gettext(), but it's quite magic. We don't need to assume
multiple layers of magic will work reliably.)
regards, tom lane
On Fri, Nov 28, 2025 at 3:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Smith <smithpb2250@gmail.com> writes:
On Fri, Nov 28, 2025 at 1:49 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
... and, probably, less ability of the compiler to verify that the
variadic arguments match the format string. I think you've taken
this a bit too far.* Or is it the use of the ternary operator to select the format
string? If that's the issue, please note that my patch only introduced
the ternary operator for the first two code fragments. The third
fragment already uses ternaries in the same way on master, so I
understood that to be an established pattern as well.Sadly, you were copying bad code that we need to fix.
I'd like to make sure I understand your concern correctly so I can
revise the patch appropriately.My concern is that we follow a coding style that the compiler can
check. Most C compilers can only verify that printf-like format
strings match the actual arguments if the format string is a constant.
So if I fat-finger the format string in this example:--- a/src/backend/replication/logical/conflict.c +++ b/src/backend/replication/logical/conflict.c @@ -383,7 +383,7 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo, if (tuple_value.len > 0) { appendStringInfoString(&tuple_value, "; "); - appendStringInfo(&tuple_value, _("existing local row %s"), + appendStringInfo(&tuple_value, _("existing local row %d"), desc); }I'll hear about it:
conflict.c: In function 'build_tuple_value_details':
conflict.c:386:38: warning: format '%d' expects argument of type 'int', but argument 3 has type 'char *' [-Wformat=]
appendStringInfo(&tuple_value, _("existing local row %d"),
^~~~~~~~~~~~~~~~~~~~~~~
conflict.c:386:36: note: in expansion of macro '_'
appendStringInfo(&tuple_value, _("existing local row %d"),
^But I don't trust the compiler to see through a ternary expression
and check (both!) format strings against the actuals.In a quick test, the gcc version I have handy does seem to be able to
do that, but I don't think we should rely on that. Format strings
are something that is way too easy to get wrong, and most of us just
expect the compiler to cross-check them for us, so coding patterns
that might silently disable such compiler warnings are best avoided.(There's also some magic going on here to allow the compiler to see
through gettext(), but it's quite magic. We don't need to assume
multiple layers of magic will work reliably.)
Got it. Thanks for the detailed reason.
Here is a revised patch v2 that removes all ternaries from the format
string decision making (including the ones that were already in
master).
======
Kind Regards,
Peter Smith.
Fujitsu Australia
Attachments:
v2-0001-Simplify-format-strings-and-remove-ternaries.patchapplication/octet-stream; name=v2-0001-Simplify-format-strings-and-remove-ternaries.patchDownload
From 1a0cc4144d126e4e83125e5a3db71290d3aab671 Mon Sep 17 00:00:00 2001
From: Peter Smith <peter.b.smith@fujitsu.com>
Date: Fri, 28 Nov 2025 15:56:28 +1100
Subject: [PATCH v2] Simplify format strings and remove ternaries
---
src/backend/replication/logical/conflict.c | 33 +++++++++++++-----------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/src/backend/replication/logical/conflict.c b/src/backend/replication/logical/conflict.c
index 1669559..c3076ed 100644
--- a/src/backend/replication/logical/conflict.c
+++ b/src/backend/replication/logical/conflict.c
@@ -381,16 +381,11 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo,
if (desc)
{
if (tuple_value.len > 0)
- {
- appendStringInfoString(&tuple_value, "; ");
- appendStringInfo(&tuple_value, _("existing local row %s"),
+ appendStringInfo(&tuple_value, _("; existing local row %s"),
desc);
- }
else
- {
appendStringInfo(&tuple_value, _("Existing local row %s"),
desc);
- }
}
}
@@ -413,14 +408,9 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo,
if (desc)
{
if (tuple_value.len > 0)
- {
- appendStringInfoString(&tuple_value, "; ");
- appendStringInfo(&tuple_value, _("remote row %s"), desc);
- }
+ appendStringInfo(&tuple_value, _("; remote row %s"), desc);
else
- {
appendStringInfo(&tuple_value, _("Remote row %s"), desc);
- }
}
}
@@ -451,16 +441,21 @@ build_tuple_value_details(EState *estate, ResultRelInfo *relinfo,
{
if (tuple_value.len > 0)
{
- appendStringInfoString(&tuple_value, "; ");
- appendStringInfo(&tuple_value, OidIsValid(replica_index)
- ? _("replica identity %s")
- : _("replica identity full %s"), desc);
+ if (OidIsValid(replica_index))
+ appendStringInfo(&tuple_value,
+ _("; replica identity %s"), desc);
+ else
+ appendStringInfo(&tuple_value,
+ _("; replica identity full %s"), desc);
}
else
{
- appendStringInfo(&tuple_value, OidIsValid(replica_index)
- ? _("Replica identity %s")
- : _("Replica identity full %s"), desc);
+ if (OidIsValid(replica_index))
+ appendStringInfo(&tuple_value,
+ _("Replica identity %s"), desc);
+ else
+ appendStringInfo(&tuple_value,
+ _("Replica identity full %s"), desc);
}
}
}
--
1.8.3.1
Hello,
So, what we're doing here is to append further row-identifying details
to an errdetail string that already contains some explanation of the
problem. That is, we have something like
DETAIL: The row to be updated was deleted.
and then we add whatever this function produces, after a newline. I
don't remember any other place in our code that does such a thing. The
result would look something like
DETAIL: The row to be updated was deleted.
Key: (1, 2); remote row (1,2,3); replica identity full (1,2,3).
or something like that, where each of the parts can begin with uppercase
or not, with a semicolon or not, depending on whether there are any
previous parts.
I wonder if it would make more sense to move this identifying
information to a CONTEXT line instead, where we won't have to care about
the uppercase.
We still have to worry about the semicolon though. Maybe we should make
that explicit in the string but given control to the translator by doing
something like
/* translator: first %s is either empty or a translated list
separator, second %s is a row descriptor */
appendStringInfo(&buf, "%sremote row %s", empty ? "" :
/* translator: this is used as a list separator */
_("; "), remote_row_description);
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Debido a que la velocidad de la luz es mucho mayor que la del sonido,
algunas personas nos parecen brillantes un minuto antes
de escuchar las pelotudeces que dicen." (Roberto Fontanarrosa)
On Fri, Nov 28, 2025 at 6:38 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
So, what we're doing here is to append further row-identifying details
to an errdetail string that already contains some explanation of the
problem. That is, we have something likeDETAIL: The row to be updated was deleted.
and then we add whatever this function produces, after a newline. I
don't remember any other place in our code that does such a thing. The
result would look something likeDETAIL: The row to be updated was deleted.
Key: (1, 2); remote row (1,2,3); replica identity full (1,2,3).or something like that, where each of the parts can begin with uppercase
or not, with a semicolon or not, depending on whether there are any
previous parts.I wonder if it would make more sense to move this identifying
information to a CONTEXT line instead, where we won't have to care about
the uppercase.
As per my understanding, we typically display the information in
context via errcontext() in error context callback functions which may
not be immediately available at the location where error happens. For
example, while applying the remote changes, the error can happen
anywhere in the heap or below layer, but we add additional apply
related information (like which apply_message was being processed when
error occurred) via apply_error_callback. So, it is not clear in the
case at hand whether we should display the information which is
available at error_site via context message.
--
With Regards,
Amit Kapila.
=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
So, what we're doing here is to append further row-identifying details
to an errdetail string that already contains some explanation of the
problem. That is, we have something like
DETAIL: The row to be updated was deleted.
and then we add whatever this function produces, after a newline. I
don't remember any other place in our code that does such a thing. The
result would look something like
DETAIL: The row to be updated was deleted.
Key: (1, 2); remote row (1,2,3); replica identity full (1,2,3).
or something like that, where each of the parts can begin with uppercase
or not, with a semicolon or not, depending on whether there are any
previous parts.
I looked into the postmaster log for subscription/t/035_conflicts.pl
to find actual examples of what this code produces, and now I think
it's got worse problems than whether the C code style is good.
The output is frankly a mess, and IMO it violates our message style
guidelines in multiple ways. Here's a sampling:
2025-11-29 11:43:03.782 EST logical replication apply worker[659868] ERROR: conflict detected on relation "public.conf_tab": conflict=multiple_unique_conflicts
2025-11-29 11:43:03.782 EST logical replication apply worker[659868] DETAIL: Key already exists in unique index "conf_tab_pkey", modified in transaction 770.
Key (a)=(2); existing local row (2, 2, 2); remote row (2, 3, 4).
Key already exists in unique index "conf_tab_b_key", modified in transaction 770.
Key (b)=(3); existing local row (3, 3, 3); remote row (2, 3, 4).
Key already exists in unique index "conf_tab_c_key", modified in transaction 770.
Key (c)=(4); existing local row (4, 4, 4); remote row (2, 3, 4).
2025-11-29 11:43:04.654 EST logical replication apply worker[659907] LOG: conflict detected on relation "public.conf_tab": conflict=update_missing
2025-11-29 11:43:04.654 EST logical replication apply worker[659907] DETAIL: Could not find the row to be updated.
Remote row (6, 7, 8); replica identity (a)=(5).
2025-11-29 11:43:05.266 EST logical replication apply worker[659942] LOG: conflict detected on relation "public.tab": conflict=delete_origin_differs
2025-11-29 11:43:05.266 EST logical replication apply worker[659942] DETAIL: Deleting the row that was modified locally in transaction 790 at 2025-11-29 11:43:05.262807-05.
Existing local row (1, 3); replica identity (a)=(1).
Where to begin?
* "conflict detected" is already presuming way too much context for
someone who's looking into the postmaster log. Here we can guess
what it's about because somebody configured this test with %b in
log_line_prefix. But without the "logical replication apply worker"
cue, are users really going to know what these messages are about?
* The business about "conflict=some_kind_of_keyword" is certainly not
per our style guidelines, and I wonder how it translates. (Checks
code ... looks like we don't even try.)
* The DETAIL messages are not complete sentences or even
approximations to them, and frankly they are just about unintelligible.
* I follow "Key already exists in unique index "foo" but not "modified
in transaction 770" --- what was modified, which side's XID is this,
and why do we think an XID will be of any help whatever? The run-on
clause is very poor English in any case. "Deleting the row that was
modified locally in transaction 790 at 2025-11-29 11:43:05.262807-05."
is better in that a transaction timestamp is much more likely to be
useful than an XID, and it's marginally better English, but it's still
not a complete sentence and it reads like context not detail.
* What am I to make of this:
"Key (a)=(55); existing local row (55, 2, 3); remote row (55, 2, 3)."
From context I guess it's trying to show me the entire rows not just
the duplicated key value, but they're not labeled adequately.
Besides, I wonder how well this output scales to non-toy tables where
the rows are kilobytes or megabytes long. Index keys can be safely
assumed to not be enormously long, but it doesn't follow that it's
sane to emit the entire row. Much less several of them in one
error message.
* The three examples I show above look like the detail messages were
written by three different people who were not talking to each other.
The initial kind-of-a-sentence part is totally different style in
each case. And why do some of the outputs put the labeled key first
and others put it last?
So I think this area desperately needs significant editorial
attention, as well as some fundamental rethinking of just what
information we should show. Perhaps using errcontext would help,
but I'm not sure. I think a large part of the problem stems from
trying to cram multiple error conditions into one ereport ... is it
possible to avoid that?
regards, tom lane
I wrote:
So I think this area desperately needs significant editorial
attention, as well as some fundamental rethinking of just what
information we should show. Perhaps using errcontext would help,
but I'm not sure. I think a large part of the problem stems from
trying to cram multiple error conditions into one ereport ... is it
possible to avoid that?
After a little bit of thought, here's a sketch of a straw-man idea.
1. The longstanding output for unique constraint violations is like
ERROR: duplicate key value violates unique constraint "foo_f1_f2_key"
DETAIL: Key (f1, f2)=(1, 2) already exists.
We could do worse than to use exactly that output (perhaps even
sharing code) and add errcontext like
CONTEXT: replicated INSERT of row with replica identity (f1, f2)=(1, 2) in table "foo"
We should drop the full-row output for the reason I gave previously,
and I do not see the value of the XID printout either. This would
also serve (after s/INSERT/UPDATE/) for unique violations during
replicated updates.
2. For no-matching-row in UPDATE or DELETE, perhaps
ERROR: row to be updated[deleted] does not exist
CONTEXT: replicated UPDATE[DELETE] of row with replica identity (f1, f2)=(1, 2) in table "foo"
3. I don't understand what delete_origin_differs means or why
it's an error condition, so I won't dare to propose new text
for that. But the new text should make the reason clear,
and I think the same errcontext still works.
4. We need to make this a separate ereport for each problematic row.
Without having looked at the code, I suspect the reason it works the
way it does now is that the process will exit after ereport(ERROR).
I don't want to completely redesign how ereport works, but maybe
we could change replication apply so that the per-row reports are
emitted as ereport(LOG), and then when we get to a place where we
want to quit, end with
ERROR: stopping replication because of previously-logged errors
which would make the consequences clearer anyway.
regards, tom lane
On Sat, Nov 29, 2025 at 11:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
After a little bit of thought, here's a sketch of a straw-man idea.
1. The longstanding output for unique constraint violations is like
ERROR: duplicate key value violates unique constraint "foo_f1_f2_key"
DETAIL: Key (f1, f2)=(1, 2) already exists.We could do worse than to use exactly that output (perhaps even
sharing code) and add errcontext likeCONTEXT: replicated INSERT of row with replica identity (f1, f2)=(1, 2) in table "foo"
We should drop the full-row output for the reason I gave previously,
and I do not see the value of the XID printout either.
The reason for displaying in this style is that, in conflicts, users
may want to define their custom resolution strategies based on
conflict_type. Sometimes they need to resolve conflicts manually as
well. To make an informed decision on which version of the row is
"correct," the human reviewer needs full context.
Example: Imagine a table storing employee data.
Node A updates John Smith’s salary from $100k to $110k.
Node B simultaneously updates John Smith’s job title from "Senior Dev"
to "Lead Dev".
If the conflict report only showed:
Conflict on PK 123. Column 'Salary' differs. Column 'Job Title' differs.
The reviewer doesn't have enough information. However, if the report
shows the full rows:
Node A Version: ID:123, Name: John Smith, Salary: $110k, Title: Senior
Dev, Dept: Engineering, LastUpdatedBy: PayrollSys
Node B Version: ID:123, Name: John Smith, Salary: $100k, Title: Lead
Dev, Dept: Engineering, LastUpdatedBy: HR_Manager
Seeing the full row allows the reviewer to see who made the change and
what else is in the row. They might decide that the HR Manager's
promotion (Title change) should take precedence, or they might realize
they need to merge the two changes manually because both are valid.
Without the full row data (like LastUpdatedBy or Dept), this decision
is impossible.
Another case where we need row data is when a value in Column A might
only be valid based on the value in Column B within the same row. For
example, CHECK Constraints: Imagine a constraint where IF status =
'Active' THEN termination_date MUST BE NULL. If a conflict arises
involving these columns, you need to see both columns simultaneously
to understand why a specific resolution might violate database rules.
We do display the entire row as DETAIL for CHECK constraints even without apply.
CREATE TABLE products (
product_no integer,
name text,
price numeric CHECK (price > 0),
discounted_price numeric CHECK (discounted_price > 0),
CHECK (price > discounted_price)
);
postgres=# insert into products values (1, 'pen', 10, 20);
ERROR: new row for relation "products" violates check constraint
"products_check"
DETAIL: Failing row contains (1, pen, 10, 20).
Also, as per the docs [1]https://www.postgresql.org/docs/devel/logical-replication-conflicts.html, "The large column values are truncated to
64 bytes." while displaying conflict information which is same as what
we do while displaying the row during CHECK constraint violation.
Additionally, we checked some other systems where they display the
entire row information [2]https://docs.oracle.com/database/timesten-18.1/TTREP/conflict.htm#TTREP634 (See, "The format of a uniqueness conflict
record is").
Currently, we display the information for multiple_unique_conflicts,
when track_commit_timestamp is on as follows:
ERROR: conflict detected on relation "public.conf_tab":
conflict=multiple_unique_conflicts
DETAIL: Key already exists in unique index "conf_tab_pkey", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30.
Key (a)=(2); existing local row (2, 2, 2); remote row (2, 3, 4).
Key already exists in unique index "conf_tab_b_key", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30.
Key (b)=(3); existing local row (3, 3, 3); remote row (2, 3, 4).
Key already exists in unique index "conf_tab_c_key", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30.
Key (c)=(4); existing local row (4, 4, 4); remote row (2, 3, 4).
CONTEXT: processing remote data for replication origin "pg_16394"
during message type "INSERT" for replication target relation
"public.conf_tab" in transaction 759, finished at 0/017A7380
The idea to split it as per your suggestion, and assuming we agree
that additional row details are required for conflict/resolution based
on my above explanation.
LOG: conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab"
DETAIL: Key already exists in unique index "conf_tab_pkey", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30.
Key (a)=(2); existing local row (2, 2, 2); remote row (2, 3, 4).
LOG: conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab"
DETAIL: Key already exists in unique index "conf_tab_b_key", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30.
Key (b)=(3); existing local row (3, 3, 3); remote row (2, 3, 4).
…
ERROR: stopping replication because of previously-logged errors
CONTEXT: processing remote data for replication origin "pg_16394"
during message type "INSERT" for replication target relation
"public.conf_tab" in transaction 759, finished at 0/017A7380
OR
DETAIL: key (a)=(2) already exists in unique index "conf_tab_pkey",
modified locally in transaction 764 at 2025-12-01
08:59:50.789608+05:30
existing local row (2, 2, 2)
remote row (2, 3, 4)
This is based on the below errdetail, we use somewhere else in the code.
Example:
ERROR: cannot drop table parent because other objects depend on it
DETAIL: constraint child1_parent_id_fkey on table child1 depends on
table parent
constraint child2_parent_id_fkey on table child2 depends on table parent
view parent_view depends on table parent
view parent_view2 depends on table parent
HINT: Use DROP ... CASCADE to drop the dependent objects too.
Steps:
CREATE TABLE parent ( id SERIAL PRIMARY KEY, name TEXT);
CREATE TABLE child1 ( id SERIAL PRIMARY KEY, parent_id INT
REFERENCES parent(id));
CREATE TABLE child2 ( id SERIAL PRIMARY KEY, parent_id INT
REFERENCES parent(id));
CREATE VIEW parent_view AS SELECT * FROM parent;
CREATE VIEW parent_view2 AS SELECT id, name FROM parent;
CREATE SEQUENCE parent_seq OWNED BY parent.id;
DROP TABLE parent;
2. For no-matching-row in UPDATE or DELETE, perhaps
ERROR: row to be updated[deleted] does not exist
CONTEXT: replicated UPDATE[DELETE] of row with replica identity (f1, f2)=(1, 2) in table "foo"3. I don't understand what delete_origin_differs means or why
it's an error condition, so I won't dare to propose new text
for that. But the new text should make the reason clear,
and I think the same errcontext still works.
You can see the information on delete_origin_differs and other
conflicts in the docs [3]https://www.postgresql.org/docs/devel/logical-replication-conflicts.html. We can discuss/decide on the message of
other conflict types, once we decide multiple_unique_conflicts.
[1]: https://www.postgresql.org/docs/devel/logical-replication-conflicts.html
[2]: https://docs.oracle.com/database/timesten-18.1/TTREP/conflict.htm#TTREP634
[3]: https://www.postgresql.org/docs/devel/logical-replication-conflicts.html
--
With Regards,
Amit Kapila.
On 2025-Dec-01, Amit Kapila wrote:
The reason for displaying in this style is that, in conflicts, users
may want to define their custom resolution strategies based on
conflict_type. Sometimes they need to resolve conflicts manually as
well. To make an informed decision on which version of the row is
"correct," the human reviewer needs full context.
I think it's odd that conflict resolution depends on log entries. I
think it would be much more valuable if conflict reporting would save
the details of the conflict to some kind of conflict logging table.
How exactly are we expecting that users would bring the data from the
log file to a database row, when they are to be merged? What happens if
there are bytea columns in the table?
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Hello, everyone!
On Wed, Dec 3, 2025 at 3:41 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
I think it's odd that conflict resolution depends on log entries. I
think it would be much more valuable if conflict reporting would save
the details of the conflict to some kind of conflict logging table.
How exactly are we expecting that users would bring the data from the
log file to a database row, when they are to be merged? What happens if
there are bytea columns in the table?
Such a proposal exists already at [0]/messages/by-id/CAA4eK1+44b3vd_OWfiaVNtjf5Njb5cek09pmKRmttBByeg0NoA@mail.gmail.com.
Also, sorry for being noisy and a little-bit offtopic but I think it
is a good moment to highlight the fact that such messages are also not
so useful because sometimes they are just clearly incorrect...
You may find details and reproducers at [1]/messages/by-id/CADzfLwXZVmbo11tFS_G2i+6TfFVwHU4VUUSeoqb+8UQfuoJs8A@mail.gmail.com and [2]/messages/by-id/CADzfLwVj9ogzrf2P_H9Xb1z8vLEzBemBxp1nC9wCg4KaOFbvmw@mail.gmail.com.
Best regards,
Mikhail.
[0]: /messages/by-id/CAA4eK1+44b3vd_OWfiaVNtjf5Njb5cek09pmKRmttBByeg0NoA@mail.gmail.com
[1]: /messages/by-id/CADzfLwXZVmbo11tFS_G2i+6TfFVwHU4VUUSeoqb+8UQfuoJs8A@mail.gmail.com
[2]: /messages/by-id/CADzfLwVj9ogzrf2P_H9Xb1z8vLEzBemBxp1nC9wCg4KaOFbvmw@mail.gmail.com
On Wed, Dec 3, 2025 at 8:11 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
On 2025-Dec-01, Amit Kapila wrote:
The reason for displaying in this style is that, in conflicts, users
may want to define their custom resolution strategies based on
conflict_type. Sometimes they need to resolve conflicts manually as
well. To make an informed decision on which version of the row is
"correct," the human reviewer needs full context.I think it's odd that conflict resolution depends on log entries. I
think it would be much more valuable if conflict reporting would save
the details of the conflict to some kind of conflict logging table.
Yes, that is more valuable, and as mentioned by Mihail, we are already
discussing the same. However, that has its own challenges, like it
needs some form of manual management by the user to clean up the
conflict_history table. Also, one can say that because table insert is
a heavy operation and more prone to failures, it is better to LOG it.
Yes, the user would require some form of script to extract the
information from the LOG, so both methods have their pros and cons. I
think we will need a configurable knob to let the user decide which
way they prefer to log the conflict information.
--
With Regards,
Amit Kapila.
On Wed, Dec 3, 2025 at 11:34 PM Mihail Nikalayeu
<mihailnikalayeu@gmail.com> wrote:
Hello, everyone!
On Wed, Dec 3, 2025 at 3:41 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
I think it's odd that conflict resolution depends on log entries. I
think it would be much more valuable if conflict reporting would save
the details of the conflict to some kind of conflict logging table.
How exactly are we expecting that users would bring the data from the
log file to a database row, when they are to be merged? What happens if
there are bytea columns in the table?Such a proposal exists already at [0].
Thanks for providing the information.
Also, sorry for being noisy and a little-bit offtopic but I think it
is a good moment to highlight the fact that such messages are also not
so useful because sometimes they are just clearly incorrect...
You may find details and reproducers at [1] and [2].
Yes, I am aware of those, but IIRC, those are due to the traditional
design choice of using DirtySnapshot during the apply-phase of logical
replication. We need to discuss that separately and come to a
conclusion.
--
With Regards,
Amit Kapila.
On Sun, Nov 30, 2025 at 10:13 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
On Sat, Nov 29, 2025 at 11:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
After a little bit of thought, here's a sketch of a straw-man idea.
1. The longstanding output for unique constraint violations is like
ERROR: duplicate key value violates unique constraint "foo_f1_f2_key"
DETAIL: Key (f1, f2)=(1, 2) already exists.We could do worse than to use exactly that output (perhaps even
sharing code) and add errcontext likeCONTEXT: replicated INSERT of row with replica identity (f1, f2)=(1, 2) in table "foo"
We should drop the full-row output for the reason I gave previously,
and I do not see the value of the XID printout either.The reason for displaying in this style is that, in conflicts, users
may want to define their custom resolution strategies based on
conflict_type. Sometimes they need to resolve conflicts manually as
well. To make an informed decision on which version of the row is
"correct," the human reviewer needs full context.Example: Imagine a table storing employee data.
Node A updates John Smith’s salary from $100k to $110k.
Node B simultaneously updates John Smith’s job title from "Senior Dev"
to "Lead Dev".
If the conflict report only showed:
Conflict on PK 123. Column 'Salary' differs. Column 'Job Title' differs.The reviewer doesn't have enough information. However, if the report
shows the full rows:Node A Version: ID:123, Name: John Smith, Salary: $110k, Title: Senior
Dev, Dept: Engineering, LastUpdatedBy: PayrollSys
Node B Version: ID:123, Name: John Smith, Salary: $100k, Title: Lead
Dev, Dept: Engineering, LastUpdatedBy: HR_ManagerSeeing the full row allows the reviewer to see who made the change and
what else is in the row. They might decide that the HR Manager's
promotion (Title change) should take precedence, or they might realize
they need to merge the two changes manually because both are valid.
Without the full row data (like LastUpdatedBy or Dept), this decision
is impossible.Another case where we need row data is when a value in Column A might
only be valid based on the value in Column B within the same row. For
example, CHECK Constraints: Imagine a constraint where IF status =
'Active' THEN termination_date MUST BE NULL. If a conflict arises
involving these columns, you need to see both columns simultaneously
to understand why a specific resolution might violate database rules.We do display the entire row as DETAIL for CHECK constraints even without apply.
CREATE TABLE products (
product_no integer,
name text,
price numeric CHECK (price > 0),
discounted_price numeric CHECK (discounted_price > 0),
CHECK (price > discounted_price)
);postgres=# insert into products values (1, 'pen', 10, 20);
ERROR: new row for relation "products" violates check constraint
"products_check"
DETAIL: Failing row contains (1, pen, 10, 20).Also, as per the docs [1], "The large column values are truncated to
64 bytes." while displaying conflict information which is same as what
we do while displaying the row during CHECK constraint violation.Additionally, we checked some other systems where they display the
entire row information [2] (See, "The format of a uniqueness conflict
record is").
I find there is a value in showing the whole before and after row
image for better investigation. Users might want to write the row
images to a separate file to avoid the server logs from being bloated
and 64 bytes limit might not be enough in practice. It might also be a
good idea to show only the different column data of remote and local
rows. There is room for improvements and the conflict history table
patch would be a good start for that anyway.
Regarding the proposed message style:
The idea to split it as per your suggestion, and assuming we agree
that additional row details are required for conflict/resolution based
on my above explanation.LOG: conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab"
DETAIL: Key already exists in unique index "conf_tab_pkey", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30.
Key (a)=(2); existing local row (2, 2, 2); remote row (2, 3, 4).
LOG: conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab"
DETAIL: Key already exists in unique index "conf_tab_b_key", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30.
Key (b)=(3); existing local row (3, 3, 3); remote row (2, 3, 4).
…
ERROR: stopping replication because of previously-logged errors
CONTEXT: processing remote data for replication origin "pg_16394"
during message type "INSERT" for replication target relation
"public.conf_tab" in transaction 759, finished at 0/017A7380
The DETAIL message still seems to violate our message style guideline.
For instance, "Key (a)=(2); existing local row (2, 2, 2); remote row
(2, 3, 4)." is not a complete sentence.
I personally find that the current approach (i.e., having multiple
detail lines in DETAIL) is more understandable since it's clear for
users why the apply worker exited. With the proposed approach, after
checking the ERROR log, the user would need to collect all relevant
(potentially straggled) LOG messages from the server logs.
If we report multiple_unique_conflicts conflicts by writing a
combination of multiple LOGs and one ERROR, probably we might want to
approach this style also for other conflict types for better
consistency.
The conflict report for multiple_unique_conflicts looks redundant to
me as it shows the whole remote row image multiple times even though
it should be the same.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
On Thu, Dec 25, 2025 at 2:40 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
On Sun, Nov 30, 2025 at 10:13 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
Regarding the proposed message style:
The idea to split it as per your suggestion, and assuming we agree
that additional row details are required for conflict/resolution based
on my above explanation.LOG: conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab"
DETAIL: Key already exists in unique index "conf_tab_pkey", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30.
Key (a)=(2); existing local row (2, 2, 2); remote row (2, 3, 4).
LOG: conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab"
DETAIL: Key already exists in unique index "conf_tab_b_key", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30.
Key (b)=(3); existing local row (3, 3, 3); remote row (2, 3, 4).
…
ERROR: stopping replication because of previously-logged errors
CONTEXT: processing remote data for replication origin "pg_16394"
during message type "INSERT" for replication target relation
"public.conf_tab" in transaction 759, finished at 0/017A7380The DETAIL message still seems to violate our message style guideline.
For instance, "Key (a)=(2); existing local row (2, 2, 2); remote row
(2, 3, 4)." is not a complete sentence.I personally find that the current approach (i.e., having multiple
detail lines in DETAIL) is more understandable since it's clear for
users why the apply worker exited. With the proposed approach, after
checking the ERROR log, the user would need to collect all relevant
(potentially straggled) LOG messages from the server logs.
Fair enough. Also, with the current approach, we don't need to repeat
the same LOG message (
conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab") again and again even though we do similar things at
other places[1][client backend] STATEMENT: drop table toast_test; [client backend] LOG: drop auto-cascades to type toast_test [client backend] STATEMENT: drop table toast_test; [client backend] LOG: drop auto-cascades to type toast_test[] [client backend] STATEMENT: drop table toast_test; [client backend] LOG: drop auto-cascades to default value for column id of table toast_test [client backend] STATEMENT: drop table toast_test; [client backend] LOG: drop auto-cascades to constraint toast_test_id_not_null on table toast_test (the STATEMENT is repeated). If we have to follow your
advice then I can think of following formats:
Format-1:
DETAIL: Key (a)=(2) already exists in unique index "conf_tab_pkey",
modified locally in transaction 764 at 2025-12-01
08:59:50.789608+05:30: local row (2, 2, 2), remote row (2, 3, 4).
Key (b)=(3) already exists in unique index "conf_tab_b_key", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30: local
row (3, 3, 3); remote row (2, 3, 4).
...
Format-2:
Key already exists in unique index "conf_tab_pkey",
modified locally in transaction 764 at 2025-12-01
08:59:50.789608+05:30: key (a)=(2), local row (2, 2, 2), remote row
(2, 3, 4).
Key already exists in unique index "conf_tab_b_key", modified
locally in transaction 764 at 2025-12-01 08:59:50.789608+05:30: key
(b)=(3), local row (3, 3, 3), remote row (2, 3, 4).
The difference between both formats is that in the first format key
values are displayed in beginning and in the second format, it is
displayed along with rows. Note, I have removed the existing word from
the existing local row as that seems implicit. BTW, do we need to use
full stop at the end of each line if we have multiple lines to display
as part of the DETAIL message. AFAICS, in similar messages [2]ERROR: role "regress_role_joe" cannot be dropped because some objects depend on it DETAIL: privileges for parameter test_oat_hooks.user_var1 privileges for parameter test_oat_hooks.super_var1 privileges for parameter test_oat_hooks.user_var2 privileges for parameter test_oat_hooks.super_var2 privileges for parameter none.such privileges for parameter another.bogus (in
regression tests), we don't use full stop.
The above proposal is somewhat related to existing message:
errdetail_internal("Registering more than maximum %u bytes allowed to
block %u: current %u bytes, adding %u bytes.",
[1]: [client backend] STATEMENT: drop table toast_test; [client backend] LOG: drop auto-cascades to type toast_test [client backend] STATEMENT: drop table toast_test; [client backend] LOG: drop auto-cascades to type toast_test[] [client backend] STATEMENT: drop table toast_test; [client backend] LOG: drop auto-cascades to default value for column id of table toast_test [client backend] STATEMENT: drop table toast_test; [client backend] LOG: drop auto-cascades to constraint toast_test_id_not_null on table toast_test
[client backend] STATEMENT: drop table toast_test;
[client backend] LOG: drop auto-cascades to type toast_test
[client backend] STATEMENT: drop table toast_test;
[client backend] LOG: drop auto-cascades to type toast_test[]
[client backend] STATEMENT: drop table toast_test;
[client backend] LOG: drop auto-cascades to default value for column
id of table toast_test
[client backend] STATEMENT: drop table toast_test;
[client backend] LOG: drop auto-cascades to constraint
toast_test_id_not_null on table toast_test
[2]: ERROR: role "regress_role_joe" cannot be dropped because some objects depend on it DETAIL: privileges for parameter test_oat_hooks.user_var1 privileges for parameter test_oat_hooks.super_var1 privileges for parameter test_oat_hooks.user_var2 privileges for parameter test_oat_hooks.super_var2 privileges for parameter none.such privileges for parameter another.bogus
ERROR: role "regress_role_joe" cannot be dropped because some objects
depend on it
DETAIL: privileges for parameter test_oat_hooks.user_var1
privileges for parameter test_oat_hooks.super_var1
privileges for parameter test_oat_hooks.user_var2
privileges for parameter test_oat_hooks.super_var2
privileges for parameter none.such
privileges for parameter another.bogus
If we report multiple_unique_conflicts conflicts by writing a
combination of multiple LOGs and one ERROR, probably we might want to
approach this style also for other conflict types for better
consistency.
makes sense, if we have any other such reports but I think the
multiple detail lines is used only for multiple_unique_conflicts type
of conlict.
The conflict report for multiple_unique_conflicts looks redundant to
me as it shows the whole remote row image multiple times even though
it should be the same.
Yeah, I think it is mostly because apart from
multiple_unique_conflicts, it won't be repeated and that makes the
code easier to write in its current form. However, you have a valid
point and we should try to eliminate duplicate information for remote
row and replica identity. How about something like:
ERROR: conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab"
DETAIL: Remote row (2, 3, 4) could not be applied by using replica
identity (a)=(5).
Key (a)=(2) already exists in unique index "conf_tab_pkey",
modified locally in transaction 764 at 2025-12-01
08:59:50.789608+05:30: existing local row (2, 2, 2).
Key (b)=(3) already exists in unique index "conf_tab_b_key",
modified locally in transaction 799 at 2025-12-02
09:00:00.123456+05:30: existing local row (3, 3, 3).
Key (c)=(4) already exists in unique index "conf_tab_c_key",
modified locally in transaction 764 at 2025-12-01
08:59:50.789608+05:30: existing local row (4, 4, 4).
And when replica identity is not applicable then we can output like:
ERROR: conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab"
DETAIL: Remote row (2, 3, 4) could not be applied.
Key (a)=(2) already exists in unique index "conf_tab_pkey",
modified locally in transaction 764 at 2025-12-01
08:59:50.789608+05:30: existing local row (2, 2, 2).
Key (b)=(3) already exists in unique index "conf_tab_b_key",
modified locally in transaction 799 at 2025-12-02
09:00:00.123456+05:30: existing local row (3, 3, 3).
Key (c)=(4) already exists in unique index "conf_tab_c_key",
modified locally in transaction 764 at 2025-12-01
08:59:50.789608+05:30: existing local row (4, 4, 4).
--
With Regards,
Amit Kapila.
Dear Amit, Sawada-san,
Fair enough. Also, with the current approach, we don't need to repeat
the same LOG message (
conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab") again and again even though we do similar things at
other places[1] (the STATEMENT is repeated). If we have to follow your
advice then I can think of following formats:
...
Basically they look good to me, but I prefer to clarify the column name for each
tuples at least once per one output. Like:
```
DETAIL: Key (a)=(2) already ... local row (a, b, c) = (2, 2, 2), remote row [(a, b, c) = ](2, 3, 4).
```
Admin can easily follow what is the exact reason why it happened.
Also, the ordering of column can be different between instances, and it may cause
misunderstanding. Currently they would be re-ordered by the subscriber-side ones,
but reader may understand by the publisher-side ones.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
On Wed, Jan 7, 2026 at 3:37 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
Dear Amit, Sawada-san,
Fair enough. Also, with the current approach, we don't need to repeat
the same LOG message (
conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab") again and again even though we do similar things at
other places[1] (the STATEMENT is repeated). If we have to follow your
advice then I can think of following formats:...
Basically they look good to me, but I prefer to clarify the column name for each
tuples at least once per one output. Like:```
DETAIL: Key (a)=(2) already ... local row (a, b, c) = (2, 2, 2), remote row [(a, b, c) = ](2, 3, 4).
```Admin can easily follow what is the exact reason why it happened.
Also, the ordering of column can be different between instances, and it may cause
misunderstanding. Currently they would be re-ordered by the subscriber-side ones,
but reader may understand by the publisher-side ones.
As shown upthread, in existing places where we display the entire row,
we don't use columns, so doesn't see why we need to be different here.
I think but we can display for RI columns.
--
With Regards,
Amit Kapila.
Dear Amit,
Fair enough. Also, with the current approach, we don't need to repeat
the same LOG message (
conflict (multiple_unique_conflicts) detected on relation
"public.conf_tab") again and again even though we do similar things at
other places[1] (the STATEMENT is repeated). If we have to follow your
advice then I can think of following formats:...
As shown upthread, in existing places where we display the entire row,
we don't use columns, so doesn't see why we need to be different here.
I think but we can display for RI columns.
Thanks for the suggestion. I've created the first draft based on the comment.
While considering and implementing, I found that worker sometimes miss to read
information for indexes and relations due to the missing permissions. Previous
style just appended key/row/replica identity information at the bottom thus it
had less impacts.
However, it needs some branches if we tried to integrate into complete statements
to avoid constructing sentences at run-time.
E.g., if we have complete information, the output can be like:
```
Key (a) = (1) already exists in unique index "tab_pkey", modified in transaction 777: local row (1, 1).
```
But if the worker cannot read the content of the index, the statement should be slightly different like:
```
Unique index "tab_pkey" rejects applying due to local row (1, 1), modified in transaction 77.
```
How do you feel? This patch may need idea to reduce lines.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Attachments:
0001-Fix-errdetail-for-logical-replication-conflict.patchapplication/octet-stream; name=0001-Fix-errdetail-for-logical-replication-conflict.patchDownload
From fbf83e3b1080d28b25ac64e4dc94cd4e33dd45bd Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <hayato@example.com>
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