On Sun, Nov 29, 2020 at 1:23 PM Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Nov 27, 2020 at 04:10:27PM +0530, Ashutosh Bapat wrote: > LSN_FORMAT_ARG expands to two comma separated arguments and is kinda > open at both ends but it's handy that way.
Agreed that's useful.
> Off list Peter Eisentraut pointed out that we can not use these macros > in elog/ereport since it creates problems for translations. He > suggested adding functions which return strings and use %s when doing > so.
What's the problem with translations? We already have equivalent stuff with INT64_FORMAT that may map to %ld or %lld. But here we have a format that's set in stone.
Peter Eisentraut explained that the translation system can not handle strings broken by macros, e.g. errmsg("foo" MACRO "bar"), since it doesn't know statically what the macro resolves to. But I am not familiar with our translation system myself. But if we allow INT64_FORMAT, LSN_FORMAT should be allowed. That makes life much easier. We do not need other functions at all.
Peter E or somebody familiar with translations can provide more information.
> The patch has two functions pg_lsn_out_internal() which takes an LSN > as input and returns a palloc'ed string containing the string > representation of LSN. This may not be suitable in performance > critical paths and also may leak memory if not freed.
I'd rather never use this flavor. An OOM could mask the actual error behind.
Hmm that's a good point. It might be useful in non-ERROR cases, merely because it avoids declaring a char array :).
No one seems to reject this idea so I will add it to the next commitfest to get more reviews esp. clarification around whether macros are enough or not.