Re: Conflict Detection and Resolution - Mailing list pgsql-hackers
From | shveta malik |
---|---|
Subject | Re: Conflict Detection and Resolution |
Date | |
Msg-id | CAJpy0uD6BXYTU2v1pQ3v4FFkGWJJaLJfsi32apNz0T1hPckP=w@mail.gmail.com Whole thread Raw |
In response to | Re: Conflict Detection and Resolution (shveta malik <shveta.malik@gmail.com>) |
Responses |
Re: Conflict Detection and Resolution
|
List | pgsql-hackers |
On Thu, Sep 26, 2024 at 2:57 PM shveta malik <shveta.malik@gmail.com> wrote: > > On Fri, Sep 20, 2024 at 8:40 AM Nisha Moond <nisha.moond412@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 next set of comments on patch001: 11) conflict.c #include "access/tableam.h" (existing) #include "replication/logicalproto.h" (added by patch002) Above 2 are not needed. The code compiles without these. I think the first one has become redundant due to inclusion of other header files which indirectly include this. 12) create_subscription.sgml: + apply_remote (enum) + This resolver applies the remote change. It can be used for insert_exists, update_exists, update_origin_differs and delete_origin_differs. It is the default resolver for insert_exists and update_exists. Wrong info, it is default for update_origin_differs and delete_origin_differs 13) alter_subscription.sgml: Synopsis: + ALTER SUBSCRIPTION name RESET CONFLICT RESOLVER FOR (conflict_type) we don't support parenthesis in the syntax. So please correct the doc. postgres=# ALTER SUBSCRIPTION sub1 RESET CONFLICT RESOLVER FOR ('insert_exists'); ERROR: syntax error at or near "(" 14) alter_subscription.sgml: + CONFLICT RESOLVER ( conflict_type [= conflict_resolver] [, ... ] ) + This clause alters either the default conflict resolvers or those set by CREATE SUBSCRIPTION. Refer to section CONFLICT RESOLVERS for the details on supported conflict_types and conflict_resolvers. + conflict_type + The conflict type being reset to its default resolver setting. For details on conflict types and their default resolvers, refer to section CONFLICT RESOLVERS a) These details seem problematic. Shouldn't we have RESET as heading similar to SKIP and then try explaining both ALL and conflict_type under that. Above seems we are trying to explain conflict_type of 'CONFLICT RESOLVER ( conflict_type [= conflict_resolver]' subcommand while giving details of RESET subcommand. b) OTOH, 'CONFLICT RESOLVER ( conflict_type [= conflict_resolver]' should have its own explanation of conflict_type and conflict_resolver parameters. 15) logical-replication.sgml: Existing: + Additional logging is triggered in various conflict scenarios, each identified as a conflict type, and the conflict statistics are collected (displayed in the pg_stat_subscription_stats view). Users have the option to configure a conflict_resolver for each conflict_type when creating a subscription. For more information on the supported conflict_types detected and conflict_resolvers, refer to section CONFLICT RESOLVERS. Suggestion: Additional logging is triggered for various conflict scenarios, each categorized by a specific conflict type, with conflict statistics being gathered and displayed in the pg_stat_subscription_stats view. Users can configure a conflict_resolver for each conflict_type when creating a subscription. For more details on the supported conflict types and corresponding conflict resolvers, refer to the section on <CONFLICT RESOLVERS>. thanks Shveta
pgsql-hackers by date: