At Fri, 23 Oct 2020 19:53:00 +1100, Peter Smith <smithpb2250@gmail.com> wrote in > On Fri, Oct 23, 2020 at 5:20 PM Kyotaro Horiguchi > <horikyota.ntt@gmail.com> wrote: > > > > At Thu, 22 Oct 2020 22:31:41 -0300, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote in > > > On 2020-Oct-22, Ashutosh Bapat wrote: > > > > > > > On Thu, 22 Oct 2020 at 14:46, Kyotaro Horiguchi <horikyota.ntt@gmail.com> > > > > wrote: > > > > > > > > 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. > > The pq_send_logicalrep_msg_type() function seemed a bit overkill to me.
Ah, yes, it is what I meant. I didn't come up with the word "overkill".
> The comment in the LogicalRepMsgType enum will sufficiently ensure > nobody is going to accidentally add any bad replication message codes. > And it's not like these are going to be changed often.
Agreed.
> Why not simply downcast your enums when calling pq_sendbyte? > There are only a few of them. > > e.g. pq_sendbyte(out, (uint8)LOGICAL_REP_MSG_STREAM_COMMIT);
If you are worried about compiler warning, that explicit cast is not required. Even if the symbol is larger than 0xff, the upper bytes are silently truncated off.
I agree with Peter that the prologue of LogicalRepMsgType is enough.
I also agree with Kyotaro, that explicit cast is unnecessary.
All this together makes the second patch useless. Removed it. Instead used Kyotaro's idea in previous mail.