From: | Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com> |
---|---|
To: | Maxim Orlov <orlovmg(at)gmail(dot)com>, Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com> |
Cc: | Aleksander Alekseev <aleksander(at)timescale(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Japin Li <japinli(at)hotmail(dot)com>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Ilya Anfimov <ilan(at)tzirechnoy(dot)com> |
Subject: | Re: XID formatting and SLRU refactorings (was: Add 64-bit XIDs into PostgreSQL 15) |
Date: | 2022-03-21 18:40:22 |
Message-ID: | 7d25a313-3f98-b6c2-1aa6-36d194a41c99@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 18.03.22 16:14, Maxim Orlov wrote:
> Here is v22. It addresses things mentioned by Tom and Kyotaro. Also
> added Aleksander's changes from v21.
The v22-0002-Update-XID-formatting-in-the-.po-files.patch is not
necessary. That is done by a separate procedure.
I'm wondering about things like this:
- psprintf("xmax %u equals or exceeds next valid transaction ID %u:%u",
- xmax,
+ psprintf("xmax %llu equals or exceeds next valid transaction ID %u:%llu",
+ (unsigned long long) xmax,
EpochFromFullTransactionId(ctx->next_fxid),
- XidFromFullTransactionId(ctx->next_fxid)));
+ (unsigned long long) XidFromFullTransactionId(ctx->next_fxid)));
This %u:%u business is basically an existing workaround for the lack of
64-bit transaction identifiers. Presumably, when those are available,
all of this will be replaced by a single number display, possibly a
single %llu. So these sites you change here will have to be touched
again, and so changing this now doesn't make sense. At least that's my
guess. Maybe there needs to be a fuller explanation of how this is
meant to be transitioned.
As a more general point, I don't like plastering these bulky casts all
over the place. Casts hide problems. For example, if we currently have
elog(LOG, "xid is %u", xid);
and then xid is changed to a 64-bit type, this will give a compiler
warning until you change the format. If we add a (long long unsigned)
cast here now and then somehow forget to change the type of xid, nothing
will warn us about that. (Note that there is also third-party code
affected by this.) Besides, these casts are quite ugly anyway, and I
don't think the solution for all time should be that we have to add
these casts just to print xids.
I think there needs to be a bit more soul searching here on how to
handle that in the future and how to transition it. I don't think
targeting this patch for PG15 is useful at this point.
From | Date | Subject | |
---|---|---|---|
Next Message | Dagfinn Ilmari Mannsåker | 2022-03-21 18:41:33 | Re: refactoring basebackup.c (zstd workers) |
Previous Message | Tom Lane | 2022-03-21 18:37:12 | Re: pgsql: Add option to use ICU as global locale provider |