Michael Paquier <mich...@paquier.xyz> writes: > On Wed, Mar 13, 2024 at 02:33:52PM +0100, Peter Eisentraut wrote: >> The 0002 patch looks sensible. It would be good to fix that, otherwise it >> could have some confusing outcomes in the future. > > You mean if we begin to use %m in future callers of > emit_tap_output_v(), hypothetically? That's a fair argument.
Yeah, developers would rightfully expect to be able to use %m with anything that takes a printf format string. Case in point: when I was first doing the conversion I did change the bail() and diag() calls in pg_regress to %m, and only while I was preparing the patch for submission did I think to check the actual implementation to see if it was safe to do so. The alternative would be to document that you can't use %m with these functions, which is silly IMO, given how simple the fix is. One minor improvement I can think of is to add a comment by the save_errno declaration noting that it's needed in order to support %m. - ilmari
