Re: Conflict Detection and Resolution

On Fri, Sep 27, 2024 at 10:44 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> > >
> > > Thanks for the review.
> > > Here is the v14 patch-set fixing review comments in [1] and [2].
> > >
> >
> > Thanks for the patches. I am reviewing patch001, it is WIP, but please
> > find initial set of comments:
> >
>

Please find the next set of comments.

16)
In pg_dump.h, there is a lot of duplication of structures from
conflict.h, we can avoid that by making below changes:
--In SubscriptionInfo(), we can have a list of ConflictTypeResolver
structure and fill the elements of the list in getSubscriptions()
simply by output of pg_subscription_conflict.
--Then in dumpSubscription() we can traverse the list to verify if the
resolver is the default one, if so, skip the dump. We can create a new
function to return whether the resolver is default or not.
--We can get rid of enum ConflictType, enum ConflictResolver,
ConflictResolverNames, ConflictTypeDefaultResolvers from pg_dump.h

17)
In describe.c, we can have an 'order by' in the query so that order is
not changed everytime we update a resolver. Please see this:

For sub1, \dRs was showing below as output for Conflict Resolvers:
insert_exists = error, update_origin_differs = apply_remote,
update_exists = error, update_missing = skip, delete_origin_differs =
apply_remote, delete_missing = skip

Once I update resolver, the order gets changed:
postgres=# ALTER SUBSCRIPTION sub1 CONFLICT RESOLVER
(insert_exists='apply_remote');
ALTER SUBSCRIPTION

\dRs:
update_origin_differs = apply_remote, update_exists = error,
update_missing = skip, delete_origin_differs = apply_remote,
delete_missing = skip, insert_exists = apply_remote

18)
Similarly after making change 16, for pg_dump too, it will be good if
we maintain the order and thus can have order-by in pg_dump's query as
well.

thanks
Shveta