RE: Conflict detection and logging in logical replication

On Wednesday, August 14, 2024 7:02 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Aug 14, 2024 at 8:05 AM Zhijie Hou (Fujitsu)
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > Here is the V14 patch.
> >
>
> Review comments:
> 1.
> ReportApplyConflict()
> {
> ...
> + ereport(elevel,
> + errcode(ERRCODE_INTEGRITY_CONSTRAINT_VIOLATION),
> + errmsg("conflict detected on relation \"%s.%s\": conflict=%s",
> + get_namespace_name(RelationGetNamespace(localrel)),
> ...
>
> Is it a good idea to use ERRCODE_INTEGRITY_CONSTRAINT_VIOLATION for
> all conflicts? I think it is okay to use for insert_exists and update_exists. The
> other error codes to consider for conflicts other than insert_exists and
> update_exists are ERRCODE_T_R_SERIALIZATION_FAILURE,
> ERRCODE_CARDINALITY_VIOLATION, ERRCODE_NO_DATA,
> ERRCODE_NO_DATA_FOUND,
> ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION,
> ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE.
>
> BTW, even for insert/update_exists, won't it better to use
> ERRCODE_UNIQUE_VIOLATION ?

Agreed. I changed the patch to use ERRCODE_UNIQUE_VIOLATION for
Insert,update_exists, and ERRCODE_T_R_SERIALIZATION_FAILURE for
other conflicts.

>
> Apart from the above, the attached contains some cosmetic changes.

Thanks. I have checked and merged the changes. Here is the V15 patch
which addressed above comments.

Best Regards,
Hou zj