On Wednesday, August 14, 2024 7:02 PM Amit Kapila <amit.kapil...@gmail.com>
wrote:
>
> On Wed, Aug 14, 2024 at 8:05 AM Zhijie Hou (Fujitsu)
> <houzj.f...@fujitsu.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
v15-0001-Detect-and-log-conflicts-in-logical-replication.patch
Description: v15-0001-Detect-and-log-conflicts-in-logical-replication.patch
