On Sun, Nov 29, 2020 at 1:23 PM Michael Paquier <mich...@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.
--
Best Wishes,
Ashutosh
