On Mon, Aug 16, 2021 at 6:24 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > Therefore, perhaps a message like "... in transaction 740 with commit > timestamp 2021-08-10 14:44:38.058174+05:30" is better in terms of > consistency with other messages? >
Yes, I think that would be more consistent. On another note, for the 0001 patch, the elog ERROR at the bottom of the logicalrep_message_type() function seems to assume that the unrecognized "action" is a printable character (with its use of %c) and also that the character is meaningful to the user in some way. But given that the compiler normally warns of an unhandled enum value when switching on an enum, such an error would most likely be when action is some int value that wouldn't be meaningful to the user (as it wouldn't be one of the LogicalRepMsgType enum values). I therefore think it would be better to use %d in that ERROR: i.e. + elog(ERROR, "invalid logical replication message type %d", action); Similar comments apply to the apply_dispatch() function (and I realise it used %c before your patch). Regards, Greg Nancarrow Fujitsu Australia
