Обсуждение: Update timezone to C99
The code in src/timezone/ still contained workarounds for not wanting to rely on C99 <stdint.h> and <inttypes.h>, even though the rest of the code uses those now. To fix that, I re-downloaded the upstream code (same version as before), applied the fixes described in src/timezone/README, except those related to the integer types, and then put back the PostgreSQL-specific code and massaged things to remove irrelevant differences. There were a few other minor and cosmetic changes that I left in to improve alignment with upstream. Patch attached. Seems to work.
Вложения
Peter Eisentraut <peter@eisentraut.org> writes:
> To fix that, I re-downloaded the upstream code (same version as before),
> applied the fixes described in src/timezone/README, except those related
> to the integer types, and then put back the PostgreSQL-specific code and
> massaged things to remove irrelevant differences. There were a few
> other minor and cosmetic changes that I left in to improve alignment
> with upstream.
Hm, I've had "re-sync TZ code with upstream" on my TODO list for
several years now. I believe there's been quite a bit of churn
upstream since tzcode2020d, some of it oriented towards this same
issue of code modernization. Maybe we should try to sync with
a newer release while we're at it.
regards, tom lane
On 12.11.25 19:02, Tom Lane wrote: > Peter Eisentraut <peter@eisentraut.org> writes: >> To fix that, I re-downloaded the upstream code (same version as before), >> applied the fixes described in src/timezone/README, except those related >> to the integer types, and then put back the PostgreSQL-specific code and >> massaged things to remove irrelevant differences. There were a few >> other minor and cosmetic changes that I left in to improve alignment >> with upstream. > > Hm, I've had "re-sync TZ code with upstream" on my TODO list for > several years now. I believe there's been quite a bit of churn > upstream since tzcode2020d, some of it oriented towards this same > issue of code modernization. Maybe we should try to sync with > a newer release while we're at it. My idea was to do this C99 adjustment first so that the differences to upstream are reduced, which would hopefully simplify updating to that newer code.
Peter Eisentraut <peter@eisentraut.org> writes:
> On 12.11.25 19:02, Tom Lane wrote:
>> Hm, I've had "re-sync TZ code with upstream" on my TODO list for
>> several years now. I believe there's been quite a bit of churn
>> upstream since tzcode2020d, some of it oriented towards this same
>> issue of code modernization. Maybe we should try to sync with
>> a newer release while we're at it.
> My idea was to do this C99 adjustment first so that the differences to
> upstream are reduced, which would hopefully simplify updating to that
> newer code.
Fair enough. I looked through the patch briefly and had a couple of
minor quibbles:
@@ -905,7 +906,7 @@ transtime(const int year, const struct rule *const rulep,
for (i = 1; i < rulep->r_week; ++i)
{
if (d + DAYSPERWEEK >=
- mon_lengths[(int) leapyear][rulep->r_mon - 1])
+ mon_lengths[leapyear][rulep->r_mon - 1])
break;
d += DAYSPERWEEK;
}
@@ -915,7 +916,7 @@ transtime(const int year, const struct rule *const rulep,
*/
value = d * SECSPERDAY;
for (i = 0; i < rulep->r_mon - 1; ++i)
- value += mon_lengths[(int) leapyear][i] * SECSPERDAY;
+ value += mon_lengths[leapyear][i] * SECSPERDAY;
break;
}
"leapyear" is bool, and I believe these casts-to-int were put in to
suppress compiler bleats about up-casting that to int. This probably
dates from when we equated bool to char, and maybe it's moot now,
but I'm not sure.
@@ -47,12 +49,15 @@ typedef int64 zic_t;
static ptrdiff_t const PTRDIFF_MAX = MAXVAL(ptrdiff_t, TYPE_BIT(ptrdiff_t));
#endif
-/*
- * The type for line numbers. In Postgres, use %d to format them; upstream
- * uses PRIdMAX but we prefer not to rely on that, not least because it
- * results in platform-dependent strings to be translated.
- */
-typedef int lineno_t;
+/* The minimum alignment of a type, for pre-C11 platforms. */
+#if __STDC_VERSION__ < 201112
+#define _Alignof(type) offsetof(struct { char a; type b; }, b)
+#endif
Since we've dropped pre-C11 support, I wonder why we'd include this
upstream workaround for that.
regards, tom lane
On 14.11.25 22:11, Tom Lane wrote:
> Peter Eisentraut <peter@eisentraut.org> writes:
>> On 12.11.25 19:02, Tom Lane wrote:
>>> Hm, I've had "re-sync TZ code with upstream" on my TODO list for
>>> several years now. I believe there's been quite a bit of churn
>>> upstream since tzcode2020d, some of it oriented towards this same
>>> issue of code modernization. Maybe we should try to sync with
>>> a newer release while we're at it.
>
>> My idea was to do this C99 adjustment first so that the differences to
>> upstream are reduced, which would hopefully simplify updating to that
>> newer code.
>
> Fair enough. I looked through the patch briefly and had a couple of
> minor quibbles:
>
> @@ -905,7 +906,7 @@ transtime(const int year, const struct rule *const rulep,
> for (i = 1; i < rulep->r_week; ++i)
> {
> if (d + DAYSPERWEEK >=
> - mon_lengths[(int) leapyear][rulep->r_mon - 1])
> + mon_lengths[leapyear][rulep->r_mon - 1])
> break;
> d += DAYSPERWEEK;
> }
> @@ -915,7 +916,7 @@ transtime(const int year, const struct rule *const rulep,
> */
> value = d * SECSPERDAY;
> for (i = 0; i < rulep->r_mon - 1; ++i)
> - value += mon_lengths[(int) leapyear][i] * SECSPERDAY;
> + value += mon_lengths[leapyear][i] * SECSPERDAY;
> break;
> }
>
> "leapyear" is bool, and I believe these casts-to-int were put in to
> suppress compiler bleats about up-casting that to int. This probably
> dates from when we equated bool to char, and maybe it's moot now,
> but I'm not sure.
Correct, with bool as char this would have fallen afoul of
-Wchar-subscripts, which is included in -Wall.
I could remove these casts as a separate patch so that the reason can be
documented more clearly.
> @@ -47,12 +49,15 @@ typedef int64 zic_t;
> static ptrdiff_t const PTRDIFF_MAX = MAXVAL(ptrdiff_t, TYPE_BIT(ptrdiff_t));
> #endif
>
> -/*
> - * The type for line numbers. In Postgres, use %d to format them; upstream
> - * uses PRIdMAX but we prefer not to rely on that, not least because it
> - * results in platform-dependent strings to be translated.
> - */
> -typedef int lineno_t;
> +/* The minimum alignment of a type, for pre-C11 platforms. */
> +#if __STDC_VERSION__ < 201112
> +#define _Alignof(type) offsetof(struct { char a; type b; }, b)
> +#endif
>
> Since we've dropped pre-C11 support, I wonder why we'd include this
> upstream workaround for that.
I figured it would be better to reduce divergences from upstream, even
if we don't need all the code.
Peter Eisentraut <peter@eisentraut.org> writes:
> On 14.11.25 22:11, Tom Lane wrote:
>> "leapyear" is bool, and I believe these casts-to-int were put in to
>> suppress compiler bleats about up-casting that to int. This probably
>> dates from when we equated bool to char, and maybe it's moot now,
>> but I'm not sure.
> Correct, with bool as char this would have fallen afoul of
> -Wchar-subscripts, which is included in -Wall.
> I could remove these casts as a separate patch so that the reason can be
> documented more clearly.
Either that or just explain it in a para of the commit message.
>> Since we've dropped pre-C11 support, I wonder why we'd include this
>> upstream workaround for that.
> I figured it would be better to reduce divergences from upstream, even
> if we don't need all the code.
[ shrug... ] Okay.
No further comments.
regards, tom lane
On 19.11.25 16:48, Tom Lane wrote: > No further comments. This has been committed. FWIW, I don't have any plans currently to work on upgrading the code to newer upstream versions.