Re: FW: Timezone library

This mail apparantly didn't make it through because it was too large.
Resending it without the largest file, tzlib.tgz. I've put this file (+
the patches) on http://www.hagander.net/pgsql/.

//Magnus

> -----Original Message-----
> From: Magnus Hagander
> Sent: Sunday, April 18, 2004 9:05 PM
> To: pgsql-patches(at)postgresql(dot)org
> Subject: Timezone library
>
>
> Hi!
>
> Attached is a slightly modified version of Arthur David
> Olson's timezone library for inclusion in postgresql, pending
> review of course. This is required to give win32 working
> timezone functions on dates before 1970. It has also been
> indicated that it might be interesting to have such a library
> on other platforms as well. Note that with this enabled, the
> timezone data files are installed in <pgdir>/share/timezone.
>
> Attached are the following files:
> * tzlib.tgz - the timezone implementation, including the
> datafiles required to build the timezone data files.
> * timezone.patch - the modifications made to existing files
> to make it work. This includes setting the FRONTEND flat for ecpg.
> * tzcode.diff - lists the modifications that I have made to
> the files from the original timezone files, for reference.
>
>
> Bruce has offered to do the autoconf and Makefile integration
> parts - I don't know autoconf enough to do that part... The
> #defines necessary are "activeted" by setting USE_PGTZ. Of
> course, the appropriate files also have to be linked in.
>
>
> With this patch applied, and the timezone data installed,
> win32 now passes all time-related regression tests (3 now
> fails, 2 "known" and one being locale-inflicted, still
> investigating that one)
>
>
> //Magnus
>
>
>
>
<<tzcode_changes.diff>> <<timezone.patch>>

I assume we want to integrate our own timezone library into PostgreSQL.
We have talked about it for a few releases, and Magnus has done it for
us because Win32 needs it for pre-1970 dates.

The patch to PostgreSQL is 100 lines of diff, and the changes to Olson's
timezone library is 100 lines of diff. The timezone library itself is
173k tgz file, which will add to our download size, but not too much.

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

---------------------------------------------------------------------------

Magnus Hagander wrote:
> This mail apparantly didn't make it through because it was too large.
> Resending it without the largest file, tzlib.tgz. I've put this file (+
> the patches) on http://www.hagander.net/pgsql/.
>
>
> //Magnus
>
>
> > -----Original Message-----
> > From: Magnus Hagander
> > Sent: Sunday, April 18, 2004 9:05 PM
> > To: pgsql-patches(at)postgresql(dot)org
> > Subject: Timezone library
> >
> >
> > Hi!
> >
> > Attached is a slightly modified version of Arthur David
> > Olson's timezone library for inclusion in postgresql, pending
> > review of course. This is required to give win32 working
> > timezone functions on dates before 1970. It has also been
> > indicated that it might be interesting to have such a library
> > on other platforms as well. Note that with this enabled, the
> > timezone data files are installed in <pgdir>/share/timezone.
> >
> > Attached are the following files:
> > * tzlib.tgz - the timezone implementation, including the
> > datafiles required to build the timezone data files.
> > * timezone.patch - the modifications made to existing files
> > to make it work. This includes setting the FRONTEND flat for ecpg.
> > * tzcode.diff - lists the modifications that I have made to
> > the files from the original timezone files, for reference.
> >
> >
> > Bruce has offered to do the autoconf and Makefile integration
> > parts - I don't know autoconf enough to do that part... The
> > #defines necessary are "activeted" by setting USE_PGTZ. Of
> > course, the appropriate files also have to be linked in.
> >
> >
> > With this patch applied, and the timezone data installed,
> > win32 now passes all time-related regression tests (3 now
> > fails, 2 "known" and one being locale-inflicted, still
> > investigating that one)
> >
> >
> > //Magnus
> >
> >
> >
> >
> <<tzcode_changes.diff>> <<timezone.patch>>

Content-Description: tzcode_changes.diff

[ Attachment, skipping... ]

Content-Description: timezone.patch

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Am Tuesday 27 April 2004 05:06 schrieb Bruce Momjian:
> I assume we want to integrate our own timezone library into PostgreSQL.
> We have talked about it for a few releases, and Magnus has done it for
> us because Win32 needs it for pre-1970 dates.
>
> The patch to PostgreSQL is 100 lines of diff, and the changes to Olson's
> timezone library is 100 lines of diff. The timezone library itself is
> 173k tgz file, which will add to our download size, but not too much.

The patch says: Note! Not integrated with makefile/autoconf. Bruce has
promised to fix this :-)

Where is the patch for that? ISTM that this should be available first or the
build will be broken?

Peter Eisentraut wrote:
> Am Tuesday 27 April 2004 05:06 schrieb Bruce Momjian:
> > I assume we want to integrate our own timezone library into PostgreSQL.
> > We have talked about it for a few releases, and Magnus has done it for
> > us because Win32 needs it for pre-1970 dates.
> >
> > The patch to PostgreSQL is 100 lines of diff, and the changes to Olson's
> > timezone library is 100 lines of diff. The timezone library itself is
> > 173k tgz file, which will add to our download size, but not too much.
>
> The patch says: Note! Not integrated with makefile/autoconf. Bruce has
> promised to fix this :-)
>
> Where is the patch for that? ISTM that this should be available first or the
> build will be broken?

I will do the integration when I apply the patch. It is a simple enable
for win32 port only at this point. If we want to use it for Unix, we
can discuss that.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

I have added this to CVS under src/timezone and have integrated this into
configure.

However, I am getting a compile error, probably because my OS has a
timezone function defined in time.h:

gcc -O2 -fno-strict-aliasing -Wall -Wmissing-prototypes
-Wmissing-declarations -O1 -Wall -Wmissing-prototypes
-Wmissing-declarations -Wpointer-arith -Wcast-align -I../../src/include
-I/usr/local/include/readline -I/usr/contrib/include -c -o localtime.o
localtime.c
In file included from private.h:91,
from localtime.c:25:
/usr/include/time.h:104: `pg_timezone' redeclared as different kind of
symbol
localtime.c:5: previous declaration of `pg_timezone'
localtime.c:203: `pg_timezone' redeclared as different kind of symbol
/usr/include/time.h:104: previous declaration of `pg_timezone'
localtime.c:933: warning: static declaration for `tzsetwall' follows
non-static
gmake: *** [localtime.o] Error 1

Fortunately only Win32 compiles this code right now.

---------------------------------------------------------------------------

Magnus Hagander wrote:
> This mail apparantly didn't make it through because it was too large.
> Resending it without the largest file, tzlib.tgz. I've put this file (+
> the patches) on http://www.hagander.net/pgsql/.
>
>
> //Magnus
>
>
> > -----Original Message-----
> > From: Magnus Hagander
> > Sent: Sunday, April 18, 2004 9:05 PM
> > To: pgsql-patches(at)postgresql(dot)org
> > Subject: Timezone library
> >
> >
> > Hi!
> >
> > Attached is a slightly modified version of Arthur David
> > Olson's timezone library for inclusion in postgresql, pending
> > review of course. This is required to give win32 working
> > timezone functions on dates before 1970. It has also been
> > indicated that it might be interesting to have such a library
> > on other platforms as well. Note that with this enabled, the
> > timezone data files are installed in <pgdir>/share/timezone.
> >
> > Attached are the following files:
> > * tzlib.tgz - the timezone implementation, including the
> > datafiles required to build the timezone data files.
> > * timezone.patch - the modifications made to existing files
> > to make it work. This includes setting the FRONTEND flat for ecpg.
> > * tzcode.diff - lists the modifications that I have made to
> > the files from the original timezone files, for reference.
> >
> >
> > Bruce has offered to do the autoconf and Makefile integration
> > parts - I don't know autoconf enough to do that part... The
> > #defines necessary are "activeted" by setting USE_PGTZ. Of
> > course, the appropriate files also have to be linked in.
> >
> >
> > With this patch applied, and the timezone data installed,
> > win32 now passes all time-related regression tests (3 now
> > fails, 2 "known" and one being locale-inflicted, still
> > investigating that one)
> >
> >
> > //Magnus
> >
> >
> >
> >
> <<tzcode_changes.diff>> <<timezone.patch>>

Content-Description: tzcode_changes.diff

[ Attachment, skipping... ]

Content-Description: timezone.patch

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> /usr/include/time.h:104: `pg_timezone' redeclared as different kind of
> symbol

Your <time.h> really defines "pg_timezone"?? I'm wondering if this is
an indirect effect of a macro naming collision.

regards, tom lane

Bruce Momjian wrote:
> I have added this to CVS under src/timezone and have integrated this
> into configure.

I think it would be better to put it under one of the generic
subdirectories, such as src/utils/timezone. We may have to add
additional libraries of this kind in the future, and it would be better
to have them grouped somehow.

Peter Eisentraut wrote:
> Bruce Momjian wrote:
> > I have added this to CVS under src/timezone and have integrated this
> > into configure.
>
> I think it would be better to put it under one of the generic
> subdirectories, such as src/utils/timezone. We may have to add
> additional libraries of this kind in the future, and it would be better
> to have them grouped somehow.

I put it under /src because it is a separate module of code that we may
need to update someday with a new release. In that way it isn't really
part of our backend binary but a module we call.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Am Freitag, 30. April 2004 15:27 schrieb Bruce Momjian:
> I put it under /src because it is a separate module of code that we may
> need to update someday with a new release. In that way it isn't really
> part of our backend binary but a module we call.

The same could be said of regex, but that has been very happy in its place.

Peter Eisentraut wrote:
> Am Freitag, 30. April 2004 15:27 schrieb Bruce Momjian:
> > I put it under /src because it is a separate module of code that we may
> > need to update someday with a new release. In that way it isn't really
> > part of our backend binary but a module we call.
>
> The same could be said of regex, but that has been very happy in its place.

True, but timezone has to install files in pgsql/timezone, and it isn't
_in_ the binary like the regex is in the binary.

Do we want a subdirectory of backend installing its own files in a
separate directory? Seems strange.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:
> Peter Eisentraut wrote:
> > Am Freitag, 30. April 2004 15:27 schrieb Bruce Momjian:
> > > I put it under /src because it is a separate module of code that we may
> > > need to update someday with a new release. In that way it isn't really
> > > part of our backend binary but a module we call.
> >
> > The same could be said of regex, but that has been very happy in its place.
>
> True, but timezone has to install files in pgsql/timezone, and it isn't
> _in_ the binary like the regex is in the binary.
>
> Do we want a subdirectory of backend installing its own files in a
> separate directory? Seems strange.

I realize backend does create stuff in share and lib and stuff. If
folks want it under backend, I can move it. You want it where?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > /usr/include/time.h:104: `pg_timezone' redeclared as different kind of
> > symbol
>
> Your <time.h> really defines "pg_timezone"?? I'm wondering if this is
> an indirect effect of a macro naming collision.

I applied the following patch to allow it to compile on Unix. I had to
mask the 'timezone' define before including "time.h".

I also pass pkglibdir into the timezone makefile like we do for fmgr so
it will know where to find the timezone database. This is probably only
an interim solution. We need a more general fix for lib/ anyway, and
this is basically the same problem (that the lib directory is compiled
into the binary and can't be moved).

With this change it compiles but I get this error:

zic -d /usr/local/pgsql/share/timezone data/africa data/antarctica
data/asia data/australasia data/europe data/northamerica
data/southamerica data/pacificnew data/etcetera data/factory
data/backward data/systemv data/solar87 data/solar88 data/solar89
"data/solar87", line 385: too many local time types (rule from
"data/solar87", line 45)

Magnus doesn't see it on Win32.

To test, define PGTZ in pg_config.h and set PGTZ=yes in Makefile.global.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> I think it would be better to put it under one of the generic
> subdirectories, such as src/utils/timezone. We may have to add
> additional libraries of this kind in the future, and it would be better
> to have them grouped somehow.

The closest precedent we have at the moment is src/backend/regex, which
is a fairly large library that we imported lock-stock-and-barrel.
However, this precedent wouldn't hold if the code needs to be usable
from non-backend code. ecpg seems to want to use the tz code ...

Right at the moment I am thinking we may end up keeping only about one
file out of the existing src/timezone import, so maybe it doesn't need
a whole subdirectory anyway?

regards, tom lane

Tom Lane wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)net> writes:
> > I think it would be better to put it under one of the generic
> > subdirectories, such as src/utils/timezone. We may have to add
> > additional libraries of this kind in the future, and it would be better
> > to have them grouped somehow.
>
> The closest precedent we have at the moment is src/backend/regex, which
> is a fairly large library that we imported lock-stock-and-barrel.
> However, this precedent wouldn't hold if the code needs to be usable
> from non-backend code. ecpg seems to want to use the tz code ...

I asked if ecpg should use the same timezone database as the server and
someone said it could just use the local one.

I was concerned ecpg would be reading structure and had to assume the
timezones split up the same way as the server.

> Right at the moment I am thinking we may end up keeping only about one
> file out of the existing src/timezone import, so maybe it doesn't need
> a whole subdirectory anyway?

Not sure, but it is easy to move.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

I am getting farther with the timezone library on Unix.

First, I realized that the share/timezone library doesn't have a
localtime file by default, so the PostgreSQL server doesn't know the
local timezone. I added that and got:

test=> select current_timestamp;
timestamptz
-------------------------------
2004-04-30 18:52:14.930852+00
(1 row)

test=> select timeofday();
timeofday
-------------------------------------
Fri Apr 30 22:52:17.686324 2004 EDT
(1 row)

(How are we going to deal with the missing localtime file?)

It is obviously seeing my local timezone, but it doesn't know that it is
+04. It is at least reading the right directory.

To test, define USE_PGTZ in pg_config.h and set USE_PGTZ=yes in Makefile.global.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073