On Wed, Jul 5, 2023 at 7:54 PM Ashutosh Bapat <ashutosh.bapat....@gmail.com> wrote: > > On Wed, Jul 5, 2023 at 7:07 PM Euler Taveira <eu...@eulerto.com> wrote: > > > > On Wed, Jul 5, 2023, at 7:56 AM, Alvaro Herrera wrote: > > > > On 2023-Jul-05, Amit Kapila wrote: > > > > > I think after returning "???" from logicalrep_message_type(), the > > > above is possible when we get the error: "invalid logical replication > > > message type "X"" from apply_dispatch(), right? If so, then what about > > > the case when we forgot to handle some message in > > > logicalrep_message_type() but handled it in apply_dispatch()? > > apply_dispatch() has a default case in switch() so it can > theoretically forget to handle some message type. I think we should > avoid default case in that function to catch missing message type in > that function. But if logicalrep_message_type() doesn't use default > case, it won't forget to handle a known message type. So what you are > suggesting is not possible. >
Right, but still I feel it would be better to return actual action. > It might happen that the upstream may send an unknown message type > that both apply_dispatch() and logicalrep_message_type() can not > handle. > > > ERROR: invalid logical replication message type "X" > > CONTEXT: processing remote data for replication origin "pg_16638" during > > message type "??? (88)" in transaction 796, finished at 0/1626698 > > > > IMO it could be confusing if we provide two representations of the same > > data (X > > and 88). Since we already provide "X" I don't think we need "88". Another > > option, is to remove "X" from apply_dispatch() and add "??? (88)" to > > logicalrep_message_type(). > > I think we don't need message type to be mentioned in the context for > an error about invalid message type. I think what needs to be done is > to set > apply_error_callback_arg.command = 0 before calling ereport in the > default case of apply_dispatch(). > apply_error_callback() will just return without providing a context. > Hmm, this looks a bit hacky to me. -- With Regards, Amit Kapila.
