Re: logicalrep_message_type throws an error

On Mon, Jul 17, 2023 at 7:54 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
>
> On 2023-Jul-17, Ashutosh Bapat wrote:
>
> > Prologue of psprintf() says
> >
> > * Errors are not returned to the caller, but are reported via elog(ERROR)
> > * in the backend, or printf-to-stderr-and-exit() in frontend builds.
> > * One should therefore think twice about using this in libpq.
> >
> > If an error occurs in psprintf(), it will throw an error which will
> > override the original error. I think we should avoid any stuff that
> > throws further errors.
>
> Ooh, yeah, this is an excellent point. I agree it would be better to
> avoid psprintf() here and anything that adds more failure modes.
>

I have tried to check whether we have such usage in any other error
callbacks. Though I haven't scrutinized each and every error callback,
I found a few of them where an error can be raised. For example,

rm_redo_error_callback()->initStringInfo()
CopyFromErrorCallback()->limit_printout_length()
shared_buffer_write_error_callback()->relpathperm()->relpathbackend()->GetRelationPath()->psprintf()

> Let's
> just do the thing in the original patch you submitted, to ensure all
> these strings are compile-time constants; that's likely the most robust.
>

So will we be okay with something like the below:

ERROR: invalid logical replication message type "??? (88)"
CONTEXT: processing remote data for replication origin "pg_16638"
during message type "???" in transaction 796, finished at
0/1626698

--
With Regards,
Amit Kapila.