On Mon, Jul 17, 2023 at 7:54 PM Alvaro Herrera <alvhe...@alvh.no-ip.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.
