Thread: Conflict detection for multiple_unique_conflicts in logical replication

Hi Hackers,
(CCing people involved in related discussions)

I am starting this thread to propose a new conflict detection type
"multiple_unique_conflicts" that identifies when an incoming row
during logical replication violates more than one UNIQUE constraint.
If multiple constraints (such as the primary key/replica identity
along with additional unique indexes) are violated for the same
incoming tuple, the apply_worker will trigger a dedicated
multiple_unique_conflicts conflict.

----
ISSUE:
----
Currently, when the apply_worker encounters an 'insert_exists'
conflict (where the incoming row conflicts with an existing row based
on the replica identity), it logs the conflict and errors out
immediately. If the user tries to resolve this manually by deleting
the conflicting row, the apply worker may fail again due to another
unique constraint violation on a different column. This forces users
to fix conflicts one by one, making resolution tedious and
inefficient.

Example:
Schema:
CREATE TABLE tab1 (col1 integer PRIMARY KEY, col2 integer UNIQUE, col3
integer UNIQUE);
  - col1 is Replica Identity.

Data:
 - on pub: (1, 11, 111)
 - on sub: 3 additional local Inserts: (2, 22, 222); (3, 33, 333); (4, 44, 444)
 - Concurrently on pub, new insert: (2, 33, 444)

When the new incoming tuple (2, 33, 444) is applied on the subscriber:
 - The apply worker first detects an 'insert_exists' conflict on col1
(primary key) and errors out.
 - If the user deletes the conflicting row (key col1=2) : (2, 22,
222), the apply worker fails again because col2=33 violates another
unique constraint for tuple (3, 33, 333);
 - If the user deletes col2=33, the apply worker fails yet again due
to tuple (4, 44, 444) because col3=444 is also unique.
Conflicts on both col2 and col3 (which are independent of each other)
are an example of a 'Multiple Unique Constraints' violation.

In such cases, users are forced to resolve conflicts one by one,
making the process slow and error-prone.

---
SOLUTION:
---
During an INSERT or UPDATE conflict check, instead of stopping at the
first encountered conflict, the apply_worker will now check all unique
indexes before reporting a conflict. If multiple unique key violations
are found, it will report a 'multiple_unique_conflicts' conflict,
listing all conflicting tuples in the logs. If only a single key
conflict is detected, the existing 'insert_exists' conflict will be
raised as it is now.

This helps users resolve conflicts in one go, by deleting all
conflicting tuples at once, instead of handling them one at a time,
unless they choose to skip the transaction.

Without an automatic resolution strategy in place, the apply worker
will keep retrying until manual intervention resolves the conflict.

Like other existing conflict types, statistics for
multiple_unique_conflicts will be collected and can be viewed in the
pg_stat_subscription_stats view for monitoring.
~~~

Note:
Beyond its immediate benefits, this conflict type also helps the
future automatic resolution [1]. Handling multiple unique conflicts
under the 'insert_exists' or 'update_exists' conflict resolutions can
cause unexpected behavior since their resolution strategies are
designed for single-tuple conflicts. For example, 'last_update_wins'
works well for a single key conflict but fails when the remote tuple
is newer than some conflicting local rows and older than others,
making resolution unclear. Introducing 'multiple_unique_conflicts'
ensures such cases are handled separately and correctly with dedicated
resolution strategies. The possible resolution strategies for this
conflict type could be:

 apply: Apply the remote change by deleting all conflicting rows and
then inserting or updating the remote row.
 skip: Skip applying the change.
 error: Error out on conflict (default behavior).
~~~

The proposal patch is attached. Suggestions and feedback are highly appreciated!

[1] https://www.postgresql.org/message-id/CAJpy0uD0-DpYVMtsxK5R%3DzszXauZBayQMAYET9sWr_w0CNWXxQ%40mail.gmail.com

--
Thanks,
Nisha

Hi Nisha.

Some review comments for patch v1-0001

======
GENERAL

1.
This may be a basic/naive question, but it is unclear to me why we
care about the stats of confl_multiple_unique_conflicts?

I can understand it would be useful to get multiple conflicts logged
at the same time so the user doesn't have to stumble across them one
by one when fixing them, but as far as the stats go, why do we care
about this stat? Also, since you don't distinguish between multiple
insert conflicts versus multiple update conflicts the stat usefulness
seemed even more dubious.

(because of this question, I skipped reviewing some parts of this
patch related to stats)

======
Commit message

2.
/reports a multiple_unique_conflicts/reports multiple_unique_conflicts/

~~~

3.
Also, the patch adds a new column in view pg_stat_subscription_stats
to support stats collection for this conflict type.

~

Maybe say the name of the added column.

======
doc/src/sgml/logical-replication.sgml

4.
+      <para>
+       Inserting a row or updating values of a row violates more than
one <literal>NOT DEFERRABLE</literal>
+       unique constraints. Note that to log the origin and commit
timestamp details of the conflicting key,
+       <link linkend="guc-track-commit-timestamp"><varname>track_commit_timestamp</varname></link>
+       should be enabled on the subscriber. In this case, an error will be
+       raised until the conflict is resolved manually.
+      </para>

/unique constraints./unique constraint./

======
doc/src/sgml/monitoring.sgml

5.
See the GENERAL question. Why is this useful?

======
src/backend/executor/execReplication.c

6.
+
+ /*
+ * Report an UPDATE_EXISTS conflict when only one unique constraint is
+ * violated.
+ */

The comment says UPDATE_EXISTS, but I thought that could be something
else like INSERT_EXISTS too.

~~~

7.
+ /*
+ * Report a MULTIPLE_UNIQUE_CONFLICTS when two or more unique constraints
+ * are violated.
+ */

/Report a MULTIPLE_UNIQUE_CONFLICTS/Report MULTIPLE_UNIQUE_CONFLICTS/

======
src/backend/replication/logical/conflict.c

ReportMultipleUniqueConflict:

8.
+ * 'conflictslots_list' is a list of slots containing the local tuples
+ * that conflict with the remote tuple.

There is no parameter called conflictslots_list

======
.../t/035_multiple_unique_conflicts.pl

9.
+# Copyright (c) 2024, PostgreSQL Global Development Group
+

/2024/2025/

~~~

10.
+my $log_location = -s $node_subscriber->logfile;
+##################################################
+# Test multiple_unique_conflicts due to INSERT
+##################################################

Add blank line above that comment.

~~~

11.
+ok( $logfile =~
+   qr/conflict detected on relation \"public.conf_tab\":
conflict=multiple_unique_conflicts*\n.*DETAIL:.* The remote tuple
violates multiple unique constraints on the local table./,
+ 'multiple_unique_conflicts detected during insertion');
+

Won't it be more useful to also log the column name causing the
conflict? Otherwise, if there are 100s of unique columns and just 2 of
them are conflicts how will the user know what to look for when fixing
it?

~~~

12.
+ok( $logfile =~
+   qr/conflict detected on relation \"public.conf_tab\":
conflict=multiple_unique_conflicts*\n.*DETAIL:.* The remote tuple
violates multiple unique constraints on the local table./,
+ 'multiple_unique_conflicts detected during update');

(same as #12)

Won't it be more useful to also log the column name causing the
conflict? Otherwise, if there are 100s of unique columns and just 2 of
them are conflicts how will the user know what to look for when fixing
it?

======
Kind Regards,
Peter Smith.
Fujitsu Australia

On Mon, Feb 24, 2025 at 7:39 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi Nisha.
>
> Some review comments for patch v1-0001
>
> ======
> GENERAL
>
> 1.
> This may be a basic/naive question, but it is unclear to me why we
> care about the stats of confl_multiple_unique_conflicts?
>
> I can understand it would be useful to get multiple conflicts logged
> at the same time so the user doesn't have to stumble across them one
> by one when fixing them, but as far as the stats go, why do we care
> about this stat? Also, since you don't distinguish between multiple
> insert conflicts versus multiple update conflicts the stat usefulness
> seemed even more dubious.
>
> (because of this question, I skipped reviewing some parts of this
> patch related to stats)
>

IMO, tracking multiple_unique_conflicts, like other conflict stats,
helps users understand their workload better, especially since in
these cases, stats aren't gathered under either insert_exists or
update_exists.
I'll wait for others' opinions too on the need for the stats in this case.

> ~~~
>
> 12.
> +ok( $logfile =~
> +   qr/conflict detected on relation \"public.conf_tab\":
> conflict=multiple_unique_conflicts*\n.*DETAIL:.* The remote tuple
> violates multiple unique constraints on the local table./,
> + 'multiple_unique_conflicts detected during update');
>
> (same as #12)
>
> Won't it be more useful to also log the column name causing the
> conflict? Otherwise, if there are 100s of unique columns and just 2 of
> them are conflicts how will the user know what to look for when fixing
> it?
>

The conflicting column details are logged. In the test case, only the
header line of the DETAIL message is compared to keep it simple.
For example, the full LOG message will look like -

ERROR:  conflict detected on relation "public.conf_tab":
conflict=multiple_unique_conflicts
DETAIL:  The remote tuple violates multiple unique constraints on the
local table.
        Key already exists in unique index "conf_tab_pkey", modified
locally in transaction 757 at 2025-02-25 14:00:56.955403+05:30.
        Key (a)=(2); existing local tuple (2, 2, 2); remote tuple (2, 3, 4).
        Key already exists in unique index "conf_tab_b_key", modified
locally in transaction 758 at 2025-02-25 14:00:56.957092+05:30.
        Key (b)=(3); existing local tuple (3, 3, 3); remote tuple (2, 3, 4).
        Key already exists in unique index "conf_tab_c_key", modified
locally in transaction 759 at 2025-02-25 14:00:56.957337+05:30.
        Key (c)=(4); existing local tuple (4, 4, 4); remote tuple (2, 3, 4).
~~~~

Addressed comments and attached the v2 patch.

--
Thanks,
Nisha

On Thu, Feb 20, 2025 at 5:01 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> Hi Hackers,
> (CCing people involved in related discussions)
>
> I am starting this thread to propose a new conflict detection type
> "multiple_unique_conflicts" that identifies when an incoming row
> during logical replication violates more than one UNIQUE constraint.
> If multiple constraints (such as the primary key/replica identity
> along with additional unique indexes) are violated for the same
> incoming tuple, the apply_worker will trigger a dedicated
> multiple_unique_conflicts conflict.
>
> ----
> ISSUE:
> ----
> Currently, when the apply_worker encounters an 'insert_exists'
> conflict (where the incoming row conflicts with an existing row based
> on the replica identity), it logs the conflict and errors out
> immediately. If the user tries to resolve this manually by deleting
> the conflicting row, the apply worker may fail again due to another
> unique constraint violation on a different column. This forces users
> to fix conflicts one by one, making resolution tedious and
> inefficient.
>
> Example:
> Schema:
> CREATE TABLE tab1 (col1 integer PRIMARY KEY, col2 integer UNIQUE, col3
> integer UNIQUE);
>   - col1 is Replica Identity.
>
> Data:
>  - on pub: (1, 11, 111)
>  - on sub: 3 additional local Inserts: (2, 22, 222); (3, 33, 333); (4, 44, 444)
>  - Concurrently on pub, new insert: (2, 33, 444)
>
> When the new incoming tuple (2, 33, 444) is applied on the subscriber:
>  - The apply worker first detects an 'insert_exists' conflict on col1
> (primary key) and errors out.
>  - If the user deletes the conflicting row (key col1=2) : (2, 22,
> 222), the apply worker fails again because col2=33 violates another
> unique constraint for tuple (3, 33, 333);
>  - If the user deletes col2=33, the apply worker fails yet again due
> to tuple (4, 44, 444) because col3=444 is also unique.
> Conflicts on both col2 and col3 (which are independent of each other)
> are an example of a 'Multiple Unique Constraints' violation.
>
> In such cases, users are forced to resolve conflicts one by one,
> making the process slow and error-prone.
>
> ---
> SOLUTION:
> ---
> During an INSERT or UPDATE conflict check, instead of stopping at the
> first encountered conflict, the apply_worker will now check all unique
> indexes before reporting a conflict. If multiple unique key violations
> are found, it will report a 'multiple_unique_conflicts' conflict,
> listing all conflicting tuples in the logs. If only a single key
> conflict is detected, the existing 'insert_exists' conflict will be
> raised as it is now.

I think it makes sense to report all the unique key conflicts for a
single row at once, rather than stopping after the first one. However,
I don't understand the need to create a new conflict type. Can't we
report multiple conflicts for the row using the existing conflict
type, if different columns in the incoming row conflict with different
existing rows?

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

On Tue, Mar 11, 2025 at 11:10 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, Feb 20, 2025 at 5:01 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > Hi Hackers,
> > (CCing people involved in related discussions)
> >
> > I am starting this thread to propose a new conflict detection type
> > "multiple_unique_conflicts" that identifies when an incoming row
> > during logical replication violates more than one UNIQUE constraint.
> > If multiple constraints (such as the primary key/replica identity
> > along with additional unique indexes) are violated for the same
> > incoming tuple, the apply_worker will trigger a dedicated
> > multiple_unique_conflicts conflict.
> >
> > ----
> > ISSUE:
> > ----
> > Currently, when the apply_worker encounters an 'insert_exists'
> > conflict (where the incoming row conflicts with an existing row based
> > on the replica identity), it logs the conflict and errors out
> > immediately. If the user tries to resolve this manually by deleting
> > the conflicting row, the apply worker may fail again due to another
> > unique constraint violation on a different column. This forces users
> > to fix conflicts one by one, making resolution tedious and
> > inefficient.
> >
> > Example:
> > Schema:
> > CREATE TABLE tab1 (col1 integer PRIMARY KEY, col2 integer UNIQUE, col3
> > integer UNIQUE);
> >   - col1 is Replica Identity.
> >
> > Data:
> >  - on pub: (1, 11, 111)
> >  - on sub: 3 additional local Inserts: (2, 22, 222); (3, 33, 333); (4, 44, 444)
> >  - Concurrently on pub, new insert: (2, 33, 444)
> >
> > When the new incoming tuple (2, 33, 444) is applied on the subscriber:
> >  - The apply worker first detects an 'insert_exists' conflict on col1
> > (primary key) and errors out.
> >  - If the user deletes the conflicting row (key col1=2) : (2, 22,
> > 222), the apply worker fails again because col2=33 violates another
> > unique constraint for tuple (3, 33, 333);
> >  - If the user deletes col2=33, the apply worker fails yet again due
> > to tuple (4, 44, 444) because col3=444 is also unique.
> > Conflicts on both col2 and col3 (which are independent of each other)
> > are an example of a 'Multiple Unique Constraints' violation.
> >
> > In such cases, users are forced to resolve conflicts one by one,
> > making the process slow and error-prone.
> >
> > ---
> > SOLUTION:
> > ---
> > During an INSERT or UPDATE conflict check, instead of stopping at the
> > first encountered conflict, the apply_worker will now check all unique
> > indexes before reporting a conflict. If multiple unique key violations
> > are found, it will report a 'multiple_unique_conflicts' conflict,
> > listing all conflicting tuples in the logs. If only a single key
> > conflict is detected, the existing 'insert_exists' conflict will be
> > raised as it is now.
>
> I think it makes sense to report all the unique key conflicts for a
> single row at once, rather than stopping after the first one. However,
> I don't understand the need to create a new conflict type. Can't we
> report multiple conflicts for the row using the existing conflict
> type, if different columns in the incoming row conflict with different
> existing rows?
>

The goal of introducing a new conflict type is to handle multiple-key
conflicts separately. It will not only allow users to apply different
resolution methods for single-key vs multi-key conflicts but will also
help them identify such cases directly from stats instead of filtering
through the LOG file.
For example, with future resolution options, 'last_update_wins' will
work well for single-key conflicts (insert_exists/update_exists) since
only one local tuple will be replaced if the remote wins. However, for
multi-key conflicts, this resolution may not be feasible. Even if
supported, resolving the conflict could result in deleting multiple
tuples, which users may want to control separately, like opting to
skip or error out in case of multi-key conflicts.

For reference, databases like EDB-PGD(BDR)[1] support a separate
conflict type for multi-key cases. OTOH, Oracle[2] and DB2[3] do not
have a separate conflict_type and always ERROR out during conflict
resolution if multiple unique constraint violations happen. However,
none of these databases use the single-key conflict_type and
resolution for the multi-key conflict cases.

We’d like to know your preference—should we always ERROR out like
Oracle/DB2, or keep it as a separate conflict_type for better control
during resolution?

[1] https://www.enterprisedb.com/docs/pgd/4/bdr/conflicts/#insert-operations-that-violate-multiple-unique-constraints
[2] https://docs.oracle.com/goldengate/c1230/gg-winux/GWUAD/configuring-conflict-detection-and-resolution.htm#GWUAD316
[3] https://www.ibm.com/docs/en/idr/11.4.0?topic=console-setting-conflict-detection-resolution

--
Thanks,
Nisha Moond

Hi Nisha,

Thanks for addressing some of my v1 comments. I confirmed they are all
ok. But,  I haven't reviewed the v2 again because I still had doubts
about the "stats" question and am waiting to see how that pans out.
Meanwhile, I had a couple more replies below.

On Tue, Feb 25, 2025 at 8:37 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Mon, Feb 24, 2025 at 7:39 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Hi Nisha.
> >
> > Some review comments for patch v1-0001
> >
> > ======
> > GENERAL
> >
> > 1.
> > This may be a basic/naive question, but it is unclear to me why we
> > care about the stats of confl_multiple_unique_conflicts?
> >
> > I can understand it would be useful to get multiple conflicts logged
> > at the same time so the user doesn't have to stumble across them one
> > by one when fixing them, but as far as the stats go, why do we care
> > about this stat? Also, since you don't distinguish between multiple
> > insert conflicts versus multiple update conflicts the stat usefulness
> > seemed even more dubious.
> >
> > (because of this question, I skipped reviewing some parts of this
> > patch related to stats)
> >
>
> IMO, tracking multiple_unique_conflicts, like other conflict stats,
> helps users understand their workload better, especially since in
> these cases, stats aren't gathered under either insert_exists or
> update_exists.

When you say "stats aren't gathered" isn't that just your
implementation choice? e.g. If an INSERT fails because it
simultaneously violates multiple (say 5) different constraints I
assumed that could be implemented as conf_insert_exists += 1 or
conf_insert_exists += 5. Isn't it just a matter of documenting the
behaviour?

> I'll wait for others' opinions too on the need for the stats in this case.
>

OK. I'll revisit this thread after I learn the outcome.

> > ~~~
> >
> > 12.
> > +ok( $logfile =~
> > +   qr/conflict detected on relation \"public.conf_tab\":
> > conflict=multiple_unique_conflicts*\n.*DETAIL:.* The remote tuple
> > violates multiple unique constraints on the local table./,
> > + 'multiple_unique_conflicts detected during update');
> >
> > (same as #12)
> >
> > Won't it be more useful to also log the column name causing the
> > conflict? Otherwise, if there are 100s of unique columns and just 2 of
> > them are conflicts how will the user know what to look for when fixing
> > it?
> >
>
> The conflicting column details are logged. In the test case, only the
> header line of the DETAIL message is compared to keep it simple.
> For example, the full LOG message will look like -
>
> ERROR:  conflict detected on relation "public.conf_tab":
> conflict=multiple_unique_conflicts
> DETAIL:  The remote tuple violates multiple unique constraints on the
> local table.
>         Key already exists in unique index "conf_tab_pkey", modified
> locally in transaction 757 at 2025-02-25 14:00:56.955403+05:30.
>         Key (a)=(2); existing local tuple (2, 2, 2); remote tuple (2, 3, 4).
>         Key already exists in unique index "conf_tab_b_key", modified
> locally in transaction 758 at 2025-02-25 14:00:56.957092+05:30.
>         Key (b)=(3); existing local tuple (3, 3, 3); remote tuple (2, 3, 4).
>         Key already exists in unique index "conf_tab_c_key", modified
> locally in transaction 759 at 2025-02-25 14:00:56.957337+05:30.
>         Key (c)=(4); existing local tuple (4, 4, 4); remote tuple (2, 3, 4).

OK, but I think the "keep it simple" approach hides all the
interesting conflict details, which means you can't even tell if the
log emits all the expected conflicts or not. Changing the test regex
to match all those local/remove tuple details in the log would be more
useful.

BTW, when I looked at the
'tmp_check/log/035_multiple_unique_conflicts_subscriber.log' I didn't
understand why there are there 3 separate
"conflict=multiple_unique_conflicts" ERRORs in the log. Since,  AFAICT
in the TAP file, there are only 2 failure test cases (e.g. due to
INSERT, and due to UPDATE). Can you explain the difference?

======
Kind Regards,
Peter Smith.
Fujitsu Australia

On Tue, Mar 11, 2025 at 2:28 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Tue, Mar 11, 2025 at 11:10 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:

> The goal of introducing a new conflict type is to handle multiple-key
> conflicts separately. It will not only allow users to apply different
> resolution methods for single-key vs multi-key conflicts but will also
> help them identify such cases directly from stats instead of filtering
> through the LOG file.

Thanks for the explanation, that makes sense.

> For example, with future resolution options, 'last_update_wins' will
> work well for single-key conflicts (insert_exists/update_exists) since
> only one local tuple will be replaced if the remote wins. However, for
> multi-key conflicts, this resolution may not be feasible. Even if
> supported, resolving the conflict could result in deleting multiple
> tuples, which users may want to control separately, like opting to
> skip or error out in case of multi-key conflicts.

> For reference, databases like EDB-PGD(BDR)[1] support a separate
> conflict type for multi-key cases. OTOH, Oracle[2] and DB2[3] do not
> have a separate conflict_type and always ERROR out during conflict
> resolution if multiple unique constraint violations happen. However,
> none of these databases use the single-key conflict_type and
> resolution for the multi-key conflict cases.
>
> We’d like to know your preference—should we always ERROR out like
> Oracle/DB2, or keep it as a separate conflict_type for better control
> during resolution?

I agree that supporting this new conflict type will provide users with
better control, particularly when doing automatic conflict resolution.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

On Wed, Mar 12, 2025 at 6:33 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi Nisha,
>
> Thanks for addressing some of my v1 comments. I confirmed they are all
> ok. But,  I haven't reviewed the v2 again because I still had doubts
> about the "stats" question and am waiting to see how that pans out.
> Meanwhile, I had a couple more replies below.
>
> On Tue, Feb 25, 2025 at 8:37 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > On Mon, Feb 24, 2025 at 7:39 AM Peter Smith <smithpb2250@gmail.com> wrote:
> > >
> > > Hi Nisha.
> > >
> > > Some review comments for patch v1-0001
> > >
> > > ======
> > > GENERAL
> > >
> > > 1.
> > > This may be a basic/naive question, but it is unclear to me why we
> > > care about the stats of confl_multiple_unique_conflicts?
> > >
> > > I can understand it would be useful to get multiple conflicts logged
> > > at the same time so the user doesn't have to stumble across them one
> > > by one when fixing them, but as far as the stats go, why do we care
> > > about this stat? Also, since you don't distinguish between multiple
> > > insert conflicts versus multiple update conflicts the stat usefulness
> > > seemed even more dubious.
> > >
> > > (because of this question, I skipped reviewing some parts of this
> > > patch related to stats)
> > >
> >
> > IMO, tracking multiple_unique_conflicts, like other conflict stats,
> > helps users understand their workload better, especially since in
> > these cases, stats aren't gathered under either insert_exists or
> > update_exists.
>
> When you say "stats aren't gathered" isn't that just your
> implementation choice? e.g. If an INSERT fails because it
> simultaneously violates multiple (say 5) different constraints I
> assumed that could be implemented as conf_insert_exists += 1 or
> conf_insert_exists += 5. Isn't it just a matter of documenting the
> behaviour?
>

Yes, if we handle multiple-key conflicts under existing single-key
conflicts, we can align the implementation and documentation
accordingly.
However, as I explained in [1], we are providing a separate conflict
type for better control of auto-resolution as well. IMO, If we provide
separate handling for multi-key cases, their stats shouldn't be
included under insert_exists or update_exists. Since these cases won't
follow single-key conflict resolution methods, merging the stats could
be confusing for users.

> > I'll wait for others' opinions too on the need for the stats in this case.
> >
>
> OK. I'll revisit this thread after I learn the outcome.
>
> > > ~~~
> > >
> > > 12.
> > > +ok( $logfile =~
> > > +   qr/conflict detected on relation \"public.conf_tab\":
> > > conflict=multiple_unique_conflicts*\n.*DETAIL:.* The remote tuple
> > > violates multiple unique constraints on the local table./,
> > > + 'multiple_unique_conflicts detected during update');
> > >
> > > (same as #12)
> > >
> > > Won't it be more useful to also log the column name causing the
> > > conflict? Otherwise, if there are 100s of unique columns and just 2 of
> > > them are conflicts how will the user know what to look for when fixing
> > > it?
> > >
> >
> > The conflicting column details are logged. In the test case, only the
> > header line of the DETAIL message is compared to keep it simple.
> > For example, the full LOG message will look like -
> >
> > ERROR:  conflict detected on relation "public.conf_tab":
> > conflict=multiple_unique_conflicts
> > DETAIL:  The remote tuple violates multiple unique constraints on the
> > local table.
> >         Key already exists in unique index "conf_tab_pkey", modified
> > locally in transaction 757 at 2025-02-25 14:00:56.955403+05:30.
> >         Key (a)=(2); existing local tuple (2, 2, 2); remote tuple (2, 3, 4).
> >         Key already exists in unique index "conf_tab_b_key", modified
> > locally in transaction 758 at 2025-02-25 14:00:56.957092+05:30.
> >         Key (b)=(3); existing local tuple (3, 3, 3); remote tuple (2, 3, 4).
> >         Key already exists in unique index "conf_tab_c_key", modified
> > locally in transaction 759 at 2025-02-25 14:00:56.957337+05:30.
> >         Key (c)=(4); existing local tuple (4, 4, 4); remote tuple (2, 3, 4).
>
> OK, but I think the "keep it simple" approach hides all the
> interesting conflict details, which means you can't even tell if the
> log emits all the expected conflicts or not. Changing the test regex
> to match all those local/remove tuple details in the log would be more
> useful.
>

Fixed. Added regex to match the conflicting index, key, local tuple
and remote tuple.

> BTW, when I looked at the
> 'tmp_check/log/035_multiple_unique_conflicts_subscriber.log' I didn't
> understand why there are there 3 separate
> "conflict=multiple_unique_conflicts" ERRORs in the log. Since,  AFAICT
> in the TAP file, there are only 2 failure test cases (e.g. due to
> INSERT, and due to UPDATE). Can you explain the difference?
>

Thanks for pointing it out. There was a log offset issue—the test
waited for the LOG entry after the apply worker errored out and
retried the conflict. I've fixed it now.

Attached the v3 patch with the above comments addressed. I've also
optimized the test case and removed unnecessary code.

[1] https://www.postgresql.org/message-id/CABdArM42thrKrgvERxyw9hhc56tM4P%2BCmjKFpU07hR_9UMM8Zg%40mail.gmail.com

--
Thanks,
Nisha Moond

Hi,

For v3, CFbot failed for the patch test case due to unstable test
steps. Truncating the subscriber table resolved the apply worker error
but sometimes caused out-of-order inserts, leading to unexpected
failures instead of a conflict error. Fixed this and updated the LOG
regex check to avoid false positives.
Note: I have broken down the full error detail message check into
multiple checks to avoid very long lines in the file. I'll see if
there's a better way to compare the full error detail in a single
check.

Attached is the v4 patch (test case changes only).

--
Thanks,
Nisha Moond

On Thu, Mar 13, 2025 at 4:30 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> Attached is the v4 patch (test case changes only).
>

Comments:
=========
1.
+ /*
+ * Report an INSERT_EXISTS or UPDATE_EXISTS conflict when only one unique
+ * constraint is violated.
+ */
+ if (conflicts == 1)
+ {
+ Oid uniqueidx;
+ RepOriginId origin;
+ TimestampTz committs;
+ TransactionId xmin;
+
+ uniqueidx = linitial_oid(conflictIndexes);
+ conflictslot = linitial(conflictSlots);
+
+ GetTupleTransactionInfo(conflictslot, &xmin, &origin, &committs);
+ ReportApplyConflict(estate, resultRelInfo, ERROR, type,
+ searchslot, conflictslot, remoteslot,
+ uniqueidx, xmin, origin, committs);
+ }
+
+ /*
+ * Report MULTIPLE_UNIQUE_CONFLICTS when two or more unique constraints
+ * are violated.
+ */
+ else if (conflicts > 1)
+ ReportMultipleUniqueConflict(estate, resultRelInfo, ERROR,
+ CT_MULTIPLE_UNIQUE_CONFLICTS,
+ searchslot, remoteslot,
+ conflictSlots, conflictIndexes);

It looks a bit odd to have different functions for one or multiple
conflicts. We can improve this coding pattern by extending the current
function ReportApplyConflict to report one or multiple conflicts
depending on the length of conflictSlots.

2. From the commit message: "Also, the patch adds a new column
'confl_multiple_unique_conflicts' in view pg_stat_subscription_stats
to support stats collection for this conflict type.". This part can be
split into a second patch. Let's try to get the core patch first.

--
With Regards,
Amit Kapila.

On Mon, Mar 17, 2025 at 3:20 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Mar 13, 2025 at 4:30 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > Attached is the v4 patch (test case changes only).
> >
>
> Comments:
> =========
> 1.
> + /*
> + * Report an INSERT_EXISTS or UPDATE_EXISTS conflict when only one unique
> + * constraint is violated.
> + */
> + if (conflicts == 1)
> + {
> + Oid uniqueidx;
> + RepOriginId origin;
> + TimestampTz committs;
> + TransactionId xmin;
> +
> + uniqueidx = linitial_oid(conflictIndexes);
> + conflictslot = linitial(conflictSlots);
> +
> + GetTupleTransactionInfo(conflictslot, &xmin, &origin, &committs);
> + ReportApplyConflict(estate, resultRelInfo, ERROR, type,
> + searchslot, conflictslot, remoteslot,
> + uniqueidx, xmin, origin, committs);
> + }
> +
> + /*
> + * Report MULTIPLE_UNIQUE_CONFLICTS when two or more unique constraints
> + * are violated.
> + */
> + else if (conflicts > 1)
> + ReportMultipleUniqueConflict(estate, resultRelInfo, ERROR,
> + CT_MULTIPLE_UNIQUE_CONFLICTS,
> + searchslot, remoteslot,
> + conflictSlots, conflictIndexes);
>
> It looks a bit odd to have different functions for one or multiple
> conflicts. We can improve this coding pattern by extending the current
> function ReportApplyConflict to report one or multiple conflicts
> depending on the length of conflictSlots.
>

Modified the code to use the existing ReportApplyConflict function.

> 2. From the commit message: "Also, the patch adds a new column
> 'confl_multiple_unique_conflicts' in view pg_stat_subscription_stats
> to support stats collection for this conflict type.". This part can be
> split into a second patch. Let's try to get the core patch first.
>

I have separated the "stats" part from the core patch and will post it
as a separate patch in the next version.

Please find the attached v5-0001 patch without the stats part.

--
Thanks,
Nisha

On Wed, Mar 19, 2025 at 11:11 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> Please find the attached v5-0001 patch without the stats part.
>

Review:
=======
1.
+ foreach_ptr(TupleTableSlot, slot, conflictSlots)
+ {
+ indexoid = lfirst_oid(list_nth_cell(conflictIndexes, conflictNum));
+
+ Assert(!OidIsValid(indexoid) ||
+    CheckRelationOidLockedByMe(indexoid, RowExclusiveLock, true));
+
+ if (!localorigin)
+ GetTupleTransactionInfo(slot, &localxmin, &localorigin, &localts);
+
+ /*
+ * Build the error detail message containing the conflicting key and
+ * tuple information. The details for each conflict will be appended
+ * to err_detail.
+ */
+ errdetail_apply_conflict(estate, relinfo, type, searchslot,
+ slot, remoteslot, indexoid,
+ localxmin, localorigin, localts, &err_detail);
+
+ conflictNum++;
+ }

Won't this get TupleTransactionInfo only for the first conflicting
tuple instead of getting it separately for each tuple?

2. We make an array/list of local tuple's origin info from the caller
and send it to this ReportApplyConflict. Additionally, we can use
do...while loop to avoid calling errdetail_apply_conflict multiple
times in the function ReportApplyConflict(). We break the loop if
there are no conflictSlots after generating the first detail message.

3. Related to the new test file 't/035_multiple_unique_conflicts.pl',
is it worth to add a separate test file only for
multiple_unique_conflicts? I see that for all other conflicts, we have
tests spread over existing tests where such conflict-related tests
were present. Shall we name it as t/035_conflicts.pl? That will allow
us to add more tests related to other conflicts like update_delete in
the same file?

--
With Regards,
Amit Kapila.

On Wed, Mar 19, 2025 at 4:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Mar 19, 2025 at 11:11 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > Please find the attached v5-0001 patch without the stats part.
> >
>
> Review:
> =======
> 1.
> + foreach_ptr(TupleTableSlot, slot, conflictSlots)
> + {
> + indexoid = lfirst_oid(list_nth_cell(conflictIndexes, conflictNum));
> +
> + Assert(!OidIsValid(indexoid) ||
> +    CheckRelationOidLockedByMe(indexoid, RowExclusiveLock, true));
> +
> + if (!localorigin)
> + GetTupleTransactionInfo(slot, &localxmin, &localorigin, &localts);
> +
> + /*
> + * Build the error detail message containing the conflicting key and
> + * tuple information. The details for each conflict will be appended
> + * to err_detail.
> + */
> + errdetail_apply_conflict(estate, relinfo, type, searchslot,
> + slot, remoteslot, indexoid,
> + localxmin, localorigin, localts, &err_detail);
> +
> + conflictNum++;
> + }
>
> Won't this get TupleTransactionInfo only for the first conflicting
> tuple instead of getting it separately for each tuple?
>

Fixed.

> 2. We make an array/list of local tuple's origin info from the caller
> and send it to this ReportApplyConflict. Additionally, we can use
> do...while loop to avoid calling errdetail_apply_conflict multiple
> times in the function ReportApplyConflict(). We break the loop if
> there are no conflictSlots after generating the first detail message.
>

Implemented the suggestions.

> 3. Related to the new test file 't/035_multiple_unique_conflicts.pl',
> is it worth to add a separate test file only for
> multiple_unique_conflicts? I see that for all other conflicts, we have
> tests spread over existing tests where such conflict-related tests
> were present. Shall we name it as t/035_conflicts.pl? That will allow
> us to add more tests related to other conflicts like update_delete in
> the same file?
>

+1, renamed the test file as t/035_conflicts.pl.

Attached is v6 patch with above comments addressed.

--
Thanks,
Nisha

On Thu, Mar 20, 2025 at 3:06 PM Nisha Moond wrote:
> 
> Attached is v6 patch with above comments addressed.

Thanks updating the patch. I have some comments:

1.

The naming style of variables changed in this function seems a bit Inconsistent
with existing ones, I feel we'd better use similar style, e.g., conflictSlots => conflictslots

I included the suggested changes in 0001.

 ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
                     ConflictType type, TupleTableSlot *searchslot,
-                    TupleTableSlot *localslot, TupleTableSlot *remoteslot,
-                    Oid indexoid, TransactionId localxmin,
-                    RepOriginId localorigin, TimestampTz localts)
+                    List *conflictSlots, TupleTableSlot *remoteslot,
+                    List *conflictIndexes, List *localxmins,
+                    List *localorigins, List *localts)

2. 

I modified the documents a bit for consistency. Please see 0001
attachment.

3. 

I have been thinking whether the codes in ReportApplyConflict() can be improved
further, e.g., avoid the extra checks in do while().

One idea could be that each caller of
ReportApplyConflict() can always pass a valid list for all
list-parameter(e.g., conflictIndexes, localxmins ...). And for the cases when the
caller could not provide a valid item, it would save an invalid value
in the list and pass it to the function.

In this approach, ReportApplyConflict() seems cleaner. I am sharing a POC diff
(0002) for reference, it can pass regression test, but I have not confirmed
every case yet.

4.

+            origin = list_nth_int(localorigins, conflictNum);
...
+            localts = lappend(localts, DatumGetPointer(Int64GetDatum(committs)));

I personally feel this could be improved, because
1) RepOriginId, being a 16-bit value, is smaller than an int, which might not
   cause issues but appears somewhat odd when storing a 32-bit value within it;
2) The approach used to store 'committs' seems inelegant (and I didn't find
   precedents).
   
An alternative approach is to introduce a new structure, ConflictTupleInfo,
containing items like slot, origin, committs, and xmin for list integration.
This way, the code could be simpler, and we can avoid the above coding. Please
see 0003 for reference. (Note that some comments in this patch could be
improved)

Best Regards,
Hou zj

On Thu, Mar 20, 2025 at 5:23 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Thu, Mar 20, 2025 at 3:06 PM Nisha Moond wrote:
> >
> > Attached is v6 patch with above comments addressed.
>
> Thanks updating the patch. I have some comments:
>
> 1.
>
> The naming style of variables changed in this function seems a bit Inconsistent
> with existing ones, I feel we'd better use similar style, e.g., conflictSlots => conflictslots
>
> I included the suggested changes in 0001.
>
>  ReportApplyConflict(EState *estate, ResultRelInfo *relinfo, int elevel,
>                                         ConflictType type, TupleTableSlot *searchslot,
> -                                       TupleTableSlot *localslot, TupleTableSlot *remoteslot,
> -                                       Oid indexoid, TransactionId localxmin,
> -                                       RepOriginId localorigin, TimestampTz localts)
> +                                       List *conflictSlots, TupleTableSlot *remoteslot,
> +                                       List *conflictIndexes, List *localxmins,
> +                                       List *localorigins, List *localts)
>
> 2.
>
> I modified the documents a bit for consistency. Please see 0001
> attachment.
>
> 3.
>
> I have been thinking whether the codes in ReportApplyConflict() can be improved
> further, e.g., avoid the extra checks in do while().
>
> One idea could be that each caller of
> ReportApplyConflict() can always pass a valid list for all
> list-parameter(e.g., conflictIndexes, localxmins ...). And for the cases when the
> caller could not provide a valid item, it would save an invalid value
> in the list and pass it to the function.
>
> In this approach, ReportApplyConflict() seems cleaner. I am sharing a POC diff
> (0002) for reference, it can pass regression test, but I have not confirmed
> every case yet.
>
> 4.
>
> +                       origin = list_nth_int(localorigins, conflictNum);
> ...
> +                       localts = lappend(localts, DatumGetPointer(Int64GetDatum(committs)));
>
> I personally feel this could be improved, because
> 1) RepOriginId, being a 16-bit value, is smaller than an int, which might not
>    cause issues but appears somewhat odd when storing a 32-bit value within it;
> 2) The approach used to store 'committs' seems inelegant (and I didn't find
>    precedents).
>
> An alternative approach is to introduce a new structure, ConflictTupleInfo,
> containing items like slot, origin,  view.committs, and xmin for list integration.
> This way, the code could be simpler, and we can avoid the above coding. Please
> see 0003 for reference. (Note that some comments in this patch could be
> improved)
>

Thanks, Hou-san, for the review and fix patches. I’ve incorporated
your suggestions.
Attached are the v7 patches, including patch 002, which implements
stats collection for 'multiple_unique_conflicts' in
pg_stat_subscription_stats.

--
Thanks,
Nisha

On Fri, Mar 21, 2025 at 12:50 PM Nisha Moond wrote:

> Thanks, Hou-san, for the review and fix patches. I’ve incorporated 
> your suggestions.
> Attached are the v7 patches, including patch 002, which implements 
> stats collection for 'multiple_unique_conflicts' in pg_stat_subscription_stats.

Thanks for updating the patches.

Here are some more comments:

1.

The comments atop of ReportApplyConflict() should be updated to reflect the
updates made to the input parameters.

2.

Add ConflictTupleInfo in typedefs.list.

3.

Few typos exiting => existing


4.

 $node_subscriber->wait_for_log(
    qr/ERROR:  conflict detected on relation \"public.conf_tab\": conflict=multiple_unique_conflicts/,

We should avoid adding the elevel('ERROR') body here as the format could be
changed depending on the log_error_verbosity. Please refer to d13ff82 for
details.

Please see attachment 0001 for the proposed changes.


5.
ok( $node_subscriber->log_contains(
        qr/Key already exists in unique index \"conf_tab_pkey\".*\n.*Key \(a\)=\(2\); existing local tuple \(2, 2, 2\);
remotetuple \(2, 3, 4\)./,
 
        $log_offset),
    'multiple_unique_conflicts detected during insertion for conf_tab_pkey (a) = (2)'
);
...
ok( $node_subscriber->log_contains(
        qr/Key already exists in unique index \"conf_tab_b_key\".*\n.*Key \(b\)=\(3\); existing local tuple \(3, 3,
3\);remote tuple \(2, 3, 4\)./,
 
        $log_offset),
    'multiple_unique_conflicts detected during insertion for conf_tab_b_key (b) = (3)'
);

Currently, different detail lines are checked in separate test cases. It would
be clearer to merge these checks, ensuring comprehensive validation
including line breaks. See attachment 0002 for the proposed changes.

6.

The document related to the conflict log format should be updated. E.g., all the
places that mentioned insert|update_exists might need to mention the new
multi-key conflict as well. And it would be better to mention there would be
multiple detail lines in the new conflict. Please see attachment 0003 for the
proposed changes.

Best Regards,
Hou zj

On Fri, Mar 21, 2025 at 1:48 PM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Fri, Mar 21, 2025 at 12:50 PM Nisha Moond wrote:
>
> > Thanks, Hou-san, for the review and fix patches. I’ve incorporated
> > your suggestions.
> > Attached are the v7 patches, including patch 002, which implements
> > stats collection for 'multiple_unique_conflicts' in pg_stat_subscription_stats.
>
> Thanks for updating the patches.
>
> Here are some more comments:
>
> 1.
>
> The comments atop of ReportApplyConflict() should be updated to reflect the
> updates made to the input parameters.
>

Right, I also noticed this and updated it in the attached. Apart from
this, I have made a number of cosmetic changes in the attached. Please
review and include it in the next version unless you see any problem
with it.

--
With Regards,
Amit Kapila.

On Fri, Mar 21, 2025 at 3:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Mar 21, 2025 at 1:48 PM Zhijie Hou (Fujitsu)
> <houzj.fnst@fujitsu.com> wrote:
> >
> > On Fri, Mar 21, 2025 at 12:50 PM Nisha Moond wrote:
> >
> > > Thanks, Hou-san, for the review and fix patches. I’ve incorporated
> > > your suggestions.
> > > Attached are the v7 patches, including patch 002, which implements
> > > stats collection for 'multiple_unique_conflicts' in pg_stat_subscription_stats.
> >
> > Thanks for updating the patches.
> >
> > Here are some more comments:
> >
> > 1.
> >
> > The comments atop of ReportApplyConflict() should be updated to reflect the
> > updates made to the input parameters.
> >
>
> Right, I also noticed this and updated it in the attached. Apart from
> this, I have made a number of cosmetic changes in the attached. Please
> review and include it in the next version unless you see any problem
> with it.
>

Thanks for the review.

I’ve incorporated the changes and also addressed Hou-san’s comments
from [1] with suggested changes.

Attached are the v8 patches.

[1]
https://www.postgresql.org/message-id/OS0PR01MB57169BA3CB76A79B30223E3894DB2%40OS0PR01MB5716.jpnprd01.prod.outlook.com

--
Thanks,
Nisha

On Fri, Mar 21, 2025 at 4:43 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> Attached are the v8 patches.
>

Thanks, the patches look good to me, so pushed.

--
With Regards,
Amit Kapila.

On Fri, 21 Mar 2025 at 16:44, Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Fri, Mar 21, 2025 at 3:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Fri, Mar 21, 2025 at 1:48 PM Zhijie Hou (Fujitsu)
> > <houzj.fnst@fujitsu.com> wrote:
> > >
> > > On Fri, Mar 21, 2025 at 12:50 PM Nisha Moond wrote:
> > >
> > > > Thanks, Hou-san, for the review and fix patches. I’ve incorporated
> > > > your suggestions.
> > > > Attached are the v7 patches, including patch 002, which implements
> > > > stats collection for 'multiple_unique_conflicts' in pg_stat_subscription_stats.
> > >
> > > Thanks for updating the patches.
> > >
> > > Here are some more comments:
> > >
> > > 1.
> > >
> > > The comments atop of ReportApplyConflict() should be updated to reflect the
> > > updates made to the input parameters.
> > >
> >
> > Right, I also noticed this and updated it in the attached. Apart from
> > this, I have made a number of cosmetic changes in the attached. Please
> > review and include it in the next version unless you see any problem
> > with it.
> >
>
> Thanks for the review.
>
> I’ve incorporated the changes and also addressed Hou-san’s comments
> from [1] with suggested changes.
>
> Attached are the v8 patches.

Shouldn't these statements be the other way:
pass('multiple_unique_conflicts detected during update');
should have been:
pass('multiple_unique_conflicts detected during insert');

and
pass('multiple_unique_conflicts detected during insert');
should have been:
pass('multiple_unique_conflicts detected during update');

if you are ok with above change, the attached patch has the change for the same.

Regards,
Vignesh

On Tue, Mar 25, 2025 at 8:51 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Fri, 21 Mar 2025 at 16:44, Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > On Fri, Mar 21, 2025 at 3:38 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Fri, Mar 21, 2025 at 1:48 PM Zhijie Hou (Fujitsu)
> > > <houzj.fnst@fujitsu.com> wrote:
> > > >
> > > > On Fri, Mar 21, 2025 at 12:50 PM Nisha Moond wrote:
> > > >
> > > > > Thanks, Hou-san, for the review and fix patches. I’ve incorporated
> > > > > your suggestions.
> > > > > Attached are the v7 patches, including patch 002, which implements
> > > > > stats collection for 'multiple_unique_conflicts' in pg_stat_subscription_stats.
> > > >
> > > > Thanks for updating the patches.
> > > >
> > > > Here are some more comments:
> > > >
> > > > 1.
> > > >
> > > > The comments atop of ReportApplyConflict() should be updated to reflect the
> > > > updates made to the input parameters.
> > > >
> > >
> > > Right, I also noticed this and updated it in the attached. Apart from
> > > this, I have made a number of cosmetic changes in the attached. Please
> > > review and include it in the next version unless you see any problem
> > > with it.
> > >
> >
> > Thanks for the review.
> >
> > I’ve incorporated the changes and also addressed Hou-san’s comments
> > from [1] with suggested changes.
> >
> > Attached are the v8 patches.
>
> Shouldn't these statements be the other way:
> pass('multiple_unique_conflicts detected during update');
> should have been:
> pass('multiple_unique_conflicts detected during insert');
>
> and
> pass('multiple_unique_conflicts detected during insert');
> should have been:
> pass('multiple_unique_conflicts detected during update');
>
> if you are ok with above change, the attached patch has the change for the same.
>

LGTM and thanks for the report. I'll push this change.

--
With Regards,
Amit Kapila.

Hi Nisha,

I saw this patch was already pushed [1], but there was one thing I
never quite understood about this feature, and I didn't find the
answer in the thread posts above.

My question: Why is there only a single new conflict type being added here?

e.g.

Conflict due to INSERT
- single conflict ==> 'insert_exists'
- multiple conflicts ==> 'multiple_unique_conflicts'

Conflict due to UPDATE
- single conflict ==> 'update_exists'
- multiple conflicts ==> 'multiple_unique_conflicts'

My point is, if it is deemed useful for a user to know if a *single*
conflict was caused by an INSERT or by an UPDATE, then why is it not
equally useful to know if *multiple* conflicts were caused by an
INSERT or by an UPDATE?

In other words, instead of just 'multiple_unique_conflicts', why
wasn't this new conflict type split into two, something like
'insert_multiple_conflicts' and 'update_multiple_conflicts'?

======
[1] https://github.com/postgres/postgres/commit/73eba5004a06a744b6b8570e42432b9e9f75997b

Kind Regards,
Peter Smith.
Fujitsu Australia

On Fri, Apr 4, 2025 at 10:41 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> My point is, if it is deemed useful for a user to know if a *single*
> conflict was caused by an INSERT or by an UPDATE, then why is it not
> equally useful to know if *multiple* conflicts were caused by an
> INSERT or by an UPDATE?
>

The reason is that users can resolve single insert_exists or
update_exists by using last_update_wins kind of resolution strategy
either manually or automatically (in the future, after we get that
patch committed). The same may not be true for multiple rows case or
at least it won't be as simple the case as for single row, one may
need to consider removing multiple rows which can lead to data
inconsistency, so we are planning an ERROR as the default resolution
startegy. This could be the reason that even BDR has a single
conflict_type for this case [1][2]. I don't deny the possibility that
in the future, one comes up with a case where separate conflict types
for these makes sense, and at that time, we can consider adding more
conflict types, but for now, there is no solid case for it that is why
we kept a single conflict type.

[1] - https://www.enterprisedb.com/docs/pgd/4/bdr/conflicts/#insert-operations-that-violate-multiple-unique-constraints
[2] - https://www.enterprisedb.com/docs/pgd/4/bdr/conflicts/#update-operations-that-violate-multiple-unique-constraints

--
With Regards,
Amit Kapila.