RE: Skipping logical replication transactions on subscriber side

From Sun, Sep 5, 2021 9:58 PM Masahiko Sawada <sawada.mshk@gmail.com>:
> On Thu, Sep 2, 2021 at 8:37 PM houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.com> wrote:
> >
> > From Mon, Aug 30, 2021 3:07 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > I've attached rebased patches. 0004 patch is not the scope of this
> > > patch. It's borrowed from another thread[1] to fix the assertion
> > > failure for newly added tests. Please review them.
> >
> > Hi,
> >
> > I reviewed the 0002 patch and have a suggestion for it.
> @@ -3672,11 +3671,12 @@ typedef enum AlterSubscriptionType  typedef
> struct AlterSubscriptionStmt  {
>         NodeTag         type;
> -       AlterSubscriptionType kind; /* ALTER_SUBSCRIPTION_SET_OPTIONS,
> etc */
> +       AlterSubscriptionType kind; /* ALTER_SUBSCRIPTION_OPTIONS, etc
> + */
>         char       *subname;            /* Name of the subscription */
>         char       *conninfo;           /* Connection string to publisher */
>         List       *publication;        /* One or more publication to
> subscribe to */
>         List       *options;            /* List of DefElem nodes */
> +       bool            isReset;                /* true if RESET option */
>  } AlterSubscriptionStmt;
> 
> It's unnatural to me that AlterSubscriptionStmt has isReset flag even in spite of
> having 'kind' indicating the command. How about having RESET comand use
> the same logic of SET as you suggested while having both
> ALTER_SUBSCRIPTION_SET_OPTIONS and
> ALTER_SUBSCRIPTION_RESET_OPTIONS?

Yes, I agree with you it will look more natural with ALTER_SUBSCRIPTION_RESET_OPTIONS.

Best regards,
Hou zj