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()? Isn't it > > better to return the 'action' from the function > > logicalrep_message_type() for unknown type? That way the information > > could be a bit better and we can easily catch the code bug as well. > > Are you suggesting that logicalrep_message_type should include the > numerical value of 'action' in the ??? message? Something like this: > > ERROR: invalid logical replication message type "X" > CONTEXT: processing remote data for replication origin "pg_16638" during > message type "??? (123)" in transaction 796, finished at 0/16266F8
Isn't this numerical value already exposed in the error message (X = 88)? In this example, it is: 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(). Opinions? -- Euler Taveira EDB https://www.enterprisedb.com/
