On Fri, Sep 27, 2024 at 10:44 AM shveta malik <shveta.ma...@gmail.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
