On Thu, Aug 26, 2021 at 12:51 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Aug 26, 2021 at 7:15 AM Greg Nancarrow <gregn4...@gmail.com> wrote: > > > > On Wed, Aug 25, 2021 at 2:22 PM Masahiko Sawada <sawada.m...@gmail.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/
