On Mon, Oct 4, 2021 at 6:01 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Fri, Oct 1, 2021 at 5:32 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Fri, Oct 1, 2021 at 6:30 AM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > > > On Fri, Oct 1, 2021 at 5:05 AM Peter Eisentraut > > > <peter.eisentr...@enterprisedb.com> wrote: > > > > > > > > On 30.09.21 07:45, Masahiko Sawada wrote: > > > > > I've attached updated patches that incorporate all comments I got so > > > > > far. Please review them. > > > > > > > > I'm uneasy about the way the xids-to-be-skipped are presented as > > > > subscriptions options, similar to settings such as "binary". I see how > > > > that is convenient, but it's not really the same thing, in how you use > > > > it, is it? Even if we share some details internally, I feel that there > > > > should be a separate syntax somehow. > > > > > > Since I was thinking that ALTER SUBSCRIPTION ... SET is used to alter > > > parameters originally set by CREATE SUBSCRIPTION, in the first several > > > version patches it added a separate syntax for this feature like ALTER > > > SUBSCRIPTION ... SET SKIP TRANSACTION xxx. But Amit was concerned > > > about an additional syntax and consistency with disable_on_error[1] > > > which is proposed by Mark Diliger[2], so I’ve changed it to a > > > subscription option. > > > > > > > Yeah, the basic idea is that this is not the only option we will > > support for taking actions on error/conflict. For example, we might > > want to disable subscriptions or allow skipping transactions based on > > XID, LSN, etc. > > I guess disabling subscriptions on error/conflict and skipping the > particular transactions are somewhat different types of functions. > Disabling subscriptions on error/conflict seems likes a setting > parameter of subscriptions. The users might want to specify this > option at creation time. >
Okay, but they can still specify it by using "On Error" syntax. > Whereas, skipping the particular transaction > is a repair function that the user might want to use on the spot in > case of a failure. I’m concerned a bit that combining these functions > to one syntax could confuse the users. > Fair enough, I was mainly trying to combine the syntax for all actions that we can take "On Error". We can allow to set them either at Create Subscription or Alter Subscription time. I think here the main point is that does this addresses Peter's concern for this Patch to use a separate syntax? Peter E., can you please confirm? Do let us know if you have something else going in your mind? -- With Regards, Amit Kapila.
