Обсуждение: Replacing time_t fields in pg_control and elsewhere

Поиск
Список
Период
Сортировка

Replacing time_t fields in pg_control and elsewhere

От
Tom Lane
Дата:
We've had some recent bad experiences with trying to interpret
pg_control on a machine where time_t is a different width from
the system that made the file, eg here
http://archives.postgresql.org/pgsql-admin/2008-01/msg00023.php
and I think there was another similar case recently.  Since the
whole world is going to be gradually converting to 64-bit time_t,
this problem is clearly going to get worse before it gets better.
So we need to replace the time_t fields in pg_control and WAL records
with something less platform-dependent.

There are two reasonable choices to change them to: TimestampTz
or pg_time_t.  I see the following pros and cons:

TimestampTz: fractional-second resolution, but since it might be
either float or int, there'd still be a problem of not being able
to print it correctly on a machine with a different integer-timestamp
setting.  On the plus side, the field width is the same both ways,
so the field-shifting problems noted above wouldn't exist.  In principle
we could know by looking at the integer-timestamp field of pg_control
how to interpret the timestamp fields.

pg_time_t: only one-second resolution.  Also, since this is typedef'd
as int64, the field-width problem comes right back to haunt us on
machines where INT64_IS_BROKEN.  On the other hand, it's not clear
that there are any such machines anymore, and furthermore such a machine
is going to have a different idea of the width of some other pg_control
fields such as system_identifier anyway.

On reflection I'm leaning to pg_time_t; its portability issues seem
less bad and I don't see huge value in recording checkpoint or
startup/shutdown times to sub-second resolution.  But I wonder if
anyone wants to argue for the other choice?

BTW, I think we should also get rid of time_t in favor of pg_time_t
(or possibly TimestampTz) in all internal APIs, so that we can get
rid of the current Windows build restriction about _USE_32BIT_TIME_T.
If we don't expose time_t anywhere then it won't matter which way
extensions are built.

There are various modules that use time_t internally to store current
or recent values of time(NULL), and it's probably all right to leave
those as-is as long as the value is not exposed outside the module.
But maybe we should convert them to pg_time_t too, just to have a
uniform coding rule "don't use time_t".  Thoughts?
        regards, tom lane


Re: Replacing time_t fields in pg_control and elsewhere

От
Zdenek Kotala
Дата:
Tom Lane napsal(a):

<snip>

> pg_time_t: only one-second resolution.  Also, since this is typedef'd
> as int64, the field-width problem comes right back to haunt us on
> machines where INT64_IS_BROKEN.  On the other hand, it's not clear
> that there are any such machines anymore, and furthermore such a machine
> is going to have a different idea of the width of some other pg_control
> fields such as system_identifier anyway.

I think one-second resolution is OK. I don't expect that currently there 
is some machine with INT64_IS_BROKEN. We can add some extra check to 
configure if we want to be sure.

<snip>

> There are various modules that use time_t internally to store current
> or recent values of time(NULL), and it's probably all right to leave
> those as-is as long as the value is not exposed outside the module.
> But maybe we should convert them to pg_time_t too, just to have a
> uniform coding rule "don't use time_t".  Thoughts?

It is good rule. We should add pg_time() function as a 
replacement/wrapper of time() function.
    Zdenek