| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Peter Smith <smithpb2250(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Simplify code building the LR conflict messages |
| Date: | 2025-12-24 21:09:54 |
| Message-ID: | CAD21AoB9_qDcwhETusvMxPPUjZz_idh_FyW142Q44yV4iX5Xug@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, Nov 30, 2025 at 10:13 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Sat, Nov 29, 2025 at 11:51 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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 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.
> >
>
> 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], "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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Matthias van de Meent | 2025-12-24 21:49:08 | Re: Fixing the btree_gist inet mess |
| Previous Message | Matthias van de Meent | 2025-12-24 20:52:18 | Re: Add --extra-dependencies and immediate data dumping for pg_dump/pg_upgrade |