On Mon, Aug 16, 2021 at 6:24 AM Masahiko Sawada <sawada.mshk@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
