Re: Conflict Detection and Resolution

On Thu, 12 Sept 2024 at 14:03, Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>
> On Tue, Sep 3, 2024 at 7:42 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Fri, 30 Aug 2024 at 11:01, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> >
> > Here is the v11 patch-set. Changes are:
>
> 1) This command crashes:
> ALTER SUBSCRIPTION name RESET CONFLICT RESOLVER FOR NULL;
> #0 __strcmp_avx2 () at ../sysdeps/x86_64/multiarch/strcmp-avx2.S:116
> #1 0x000055c67270600a in ResetConflictResolver (subid=16404,
> conflict_type=0x0) at conflict.c:744
> #2 0x000055c67247e0c3 in AlterSubscription (pstate=0x55c6748ff9d0,
> stmt=0x55c67497dfe0, isTopLevel=true) at subscriptioncmds.c:1664
>
> + | ALTER SUBSCRIPTION name RESET CONFLICT
> RESOLVER FOR conflict_type
> + {
> + AlterSubscriptionStmt *n =
> + makeNode(AlterSubscriptionStmt);
> +
> + n->kind =
> ALTER_SUBSCRIPTION_RESET_CONFLICT_RESOLVER;
> + n->subname = $3;
> + n->conflict_type = $8;
> + $$ = (Node *) n;
> + }
> + ;
> +conflict_type:
> + Sconst
> { $$ = $1; }
> + | NULL_P
> { $$ = NULL; }
> ;
>
> May be conflict_type should be changed to:
> +conflict_type:
> + Sconst
> { $$ = $1; }
> ;
>
>
> Fixed.
>

Few comments:
1) This should be in (fout->remoteVersion >= 180000) check to support
dumping backward compatible server objects, else dump with older
version will fail:
+ /* Populate conflict type fields using the new query */
+ confQuery = createPQExpBuffer();
+ appendPQExpBuffer(confQuery,
+ "SELECT confrtype,
confrres FROM pg_catalog.pg_subscription_conflict "
+ "WHERE confsubid =
%u;", subinfo[i].dobj.catId.oid);
+ confRes = ExecuteSqlQuery(fout, confQuery->data,
PGRES_TUPLES_OK);
+
+ ntuples = PQntuples(confRes);
+ for (j = 0; j < ntuples; j++)

2) Can we check and throw an error before the warning is logged in
this case as it seems strange to throw a warning first and then an
error for the same track_commit_timestamp configuration:
postgres=# create subscription sub1 connection ... publication pub1
conflict resolver (insert_exists = 'last_update_wins');
WARNING: conflict detection and resolution could be incomplete due to
disabled track_commit_timestamp
DETAIL: Conflicts update_origin_differs and delete_origin_differs
cannot be detected, and the origin and commit timestamp for the local
row will not be logged.
ERROR: resolver last_update_wins requires "track_commit_timestamp" to
be enabled
HINT: Make sure the configuration parameter "track_commit_timestamp" is set.

Regards,
Vignesh