We shouldn't have the default: in the switch() block in apply_dispatch(). That prevents compilers from checking completeness. The content of the default: should be moved out to after the switch() block.
> 0002 adds wrappers on top of pq_sendbyte() and pq_getmsgbyte() to send and > receive a logical replication message type respectively. These wrappers add > more protection to make sure that the enum definitions fit one byte. This > also removes the default case from apply_dispatch() so that we can detect > any LogicalRepMsgType not handled by that function.
pg_send_logicalrep_msg_type() looks somewhat too-much. If we need something like that we shouldn't do this refactoring, I think.
Enum is an integer, and we want to send byte. The function asserts that the enum fits a byte. If there's a way to declare byte long enums I would use that. But I didn't find a way to do that.
pg_get_logicalrep_msg_type() seems doing the same check (that the value is compared aganst every keyword value) with apply_dispatch(). Why do we need that function separately from apply_dispatch()?
The second patch removes the default case you quoted above. I think that's important to detect any unhandled case at compile time rather than at run time. But we need some way to detect whether the values we get from wire are legit. pg_get_logicalrep_msg_type() does that. Further that function can be used at places other than apply_dispatch() if required without each of those places having their own validation.
