Re: Skipping logical replication transactions on subscriber side

On Thu, Aug 26, 2021 at 12:51 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Aug 26, 2021 at 7:15 AM Greg Nancarrow <gregn4422(at)gmail(dot)com> wrote:
> >
> > On Wed, Aug 25, 2021 at 2:22 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > Attached updated version patches. Please review them.
> > >
> >
> > Regarding the v11-0001 patch, it looks OK to me, but I do have one point:
> > In apply_dispatch(), wouldn't it be better to NOT move the error
> > reporting for an invalid message type into the switch as the default
> > case - because then, if you add a new message type, you won't get a
> > compiler warning (when warnings are enabled) for a missing switch
> > case, which is a handy way to alert you that the new message type
> > needs to be added as a case to the switch.
> >
>
> Do you have any suggestions on how to achieve that without adding some
> additional variable? I think it is not a very hard requirement as we
> don't follow the same at other places in code.

Yeah, I agree that it's a handy way to detect missing a switch case
but I think that we don't necessarily need it in this case. Because
there are many places in the code where doing similar things and when
it comes to apply_dispatch() it's the entry function to handle the
incoming message so it will be unlikely that we miss adding a switch
case until the patch gets committed. If we don't move it, we would end
up either adding the code resetting the
apply_error_callback_arg.command to every message type, adding a flag
indicating the message is handled and checking later, or having a big
if statement checking if the incoming message type is valid etc.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/