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
