Re: making relfilenodes 56 bits

On 2022-Jul-30, Dilip Kumar wrote:

> On Thu, Jul 28, 2022 at 9:29 PM Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:

> > Please don't do this; rather use %llu and cast to (long long).
> > Otherwise the string becomes mangled for translation.
>
> Okay, actually I did not understand the clear logic of when to use
> %llu and to use (U)INT64_FORMAT. They are both used for 64-bit
> integers. So do you think it is fine to replace all INT64_FORMAT in
> my patch with %llu?

The point here is that there are two users of the source code: one is
the compiler, and the other is gettext, which extracts the string for
the translation catalog. The compiler is OK with UINT64_FORMAT, of
course (because the preprocessor deals with it). But gettext is quite
stupid and doesn't understand that UINT64_FORMAT expands to some
specifier, so it truncates the string at the double quote sign just
before; in other words, it just doesn't work. So whenever you have a
string that ends up in a translation catalog, you must not use
UINT64_FORMAT or any other preprocessor macro; it has to be a straight
specifier in the format string.

We have found that the most convenient notation is to use %llu in the
string and cast the argument to (unsigned long long), so our convention
is to use that.

For strings that do not end up in a translation catalog, there's no
reason to use %llu-and-cast; UINT64_FORMAT is okay.

> > > @@ -2378,7 +2378,7 @@ verifyBackupPageConsistency(XLogReaderState *record)
> > > if (memcmp(replay_image_masked, primary_image_masked, BLCKSZ) != 0)
> > > {
> > > elog(FATAL,
> > > - "inconsistent page found, rel %u/%u/%u, forknum %u, blkno %u",
> > > + "inconsistent page found, rel %u/%u/" INT64_FORMAT ", forknum %u, blkno %u",
> > > rlocator.spcOid, rlocator.dbOid, rlocator.relNumber,
> > > forknum, blkno);
> >
> > Should this one be an ereport, and thus you do need to change it to that
> > and handle it like that?
>
> Okay, so you mean irrespective of this patch should this be converted
> to ereport?

Yes, I think this should be an ereport with errcode(ERRCODE_DATA_CORRUPTED).

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/