On 2023-Jul-05, Alvaro Herrera wrote: > On 2023-Jul-05, Euler Taveira wrote: > > > 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". > > The CONTEXT message could theoretically appear in other error throws, > not just in "invalid logical replication message". So the duplicity is > not really a problem.
Ah, but you're thinking that if the message type is invalid, then it will have been rejected in the "invalid logical replication message" stage, so no invalid message type will be reported. I guess there's a point to that argument as well. However, I don't see that having the numerical ASCII value there causes any harm, even if the char value is already exposed in the other message. (I'm sure you'll agree that this is quite a minor issue.) I doubt that each of these two prints of the enum value showing different formats is confusing. Yes, the enum is defined in terms of char literals, but if an actually invalid message shows up with an uint32 value outside the printable ASCII range, the printout might be ugly or useless. > > Another option, is to remove "X" from apply_dispatch() and add "??? > > (88)" to logicalrep_message_type(). Now *this* would be an actively bad idea IMO. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
