Re: Simplify code building the LR conflict messages - Mailing list pgsql-hackers

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/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.
>

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:

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] (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

[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

> 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.