FW: Timezone library

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@postgresql.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@candle.pha.pa.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@candle.pha.pa.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@postgresql.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@candle.pha.pa.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@candle.pha.pa.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.

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

Hi all,

looks like I am trying to hit a fast moving target, but here is a patch for
the changes I needed in order to get HEAD to build under win32/mingw after
these recent timezone lib changes.

I'm not sure how to enable USE_PGTZ, so for now I'm simply adding it to
CPPFLAGS in Makefile.global after configuring.

Presumably this'll be added to configure.in in due course. Alternatively, if
it is decided to have this lib available on all platforms, perhaps there
will be a configure option, in which case the provided patch would need to
reference some new flag, as opposed to "ifeq ($(PORTNAME), win32)".

Two other comments:

* timezone/private.h looks like it should be picking up its constants in
another way (ie. had to comment out HAVE_SYMLINK). Looks like we'll need to
spend some time in this file if we want it platform independent.

* pgwin32_TZDIR has the same hack as dfmgr.c:substitute_libpath_macro.
Please note that a "generalised" win32 solution to this was rejected in a
previous thread
(http://archives.postgresql.org/pgsql-patches/2004-03/msg00365.php), as it
was viewed as both restrictive (having to maintain a fixed PostgreSQL
subdirectory structure, which IMHO is not an onerous restriction for win32;
if anything it is expected by win32 users, and could simplify our installer)
and it was also suggested that a platform independent solution would be
preferable. However, to date I have seen no other solutions or suggestions.
As it is, we currently have the worst of both worlds, in that we have two
places in the code to maintain this kludge, a few more places that need it
before we can go beta, *and* it requires a fixed subdirectory structure.
Unless someone has any better ideas, I'm going to suggest resurrecting this
patch...

Cheers,
Claudio

--- 
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see 
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em
ailpolicy.html</a>

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@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

* timezone/private.h looks like it should be picking up its
constants in another way (ie. had to comment out
HAVE_SYMLINK). Looks like we'll need to spend some time in
this file if we want it platform independent.

Yes, that's the place that the tz lib stores these things. Though in
that case they set it in Makefiles, and private.h basically sets
defaults.

* pgwin32_TZDIR has the same hack as
dfmgr.c:substitute_libpath_macro.
Please note that a "generalised" win32 solution to this was
rejected in a previous thread
(http://archives.postgresql.org/pgsql-patches/2004-03/msg00365
.php), as it was viewed as both restrictive (having to
maintain a fixed PostgreSQL subdirectory structure, which
IMHO is not an onerous restriction for win32; if anything it
is expected by win32 users, and could simplify our installer)
and it was also suggested that a platform independent
solution would be preferable. However, to date I have seen no
other solutions or suggestions.

Yes, I went with this solution simply because it was there already. Once
there is agreement on how to do it in the other places, this one should
be fixed with the same code.

//Magnus

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@candle.pha.pa.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@candle.pha.pa.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@candle.pha.pa.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@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

I fixed the problem with ZIC compile needing libpgport, and the symlink
define.

I am confused why you are linking the timezone library into pg_dump and
pg_controldata. Is that required?

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

Claudio Natoli wrote:

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

Hi all,

looks like I am trying to hit a fast moving target, but here is a patch for
the changes I needed in order to get HEAD to build under win32/mingw after
these recent timezone lib changes.

I'm not sure how to enable USE_PGTZ, so for now I'm simply adding it to
CPPFLAGS in Makefile.global after configuring.

Presumably this'll be added to configure.in in due course. Alternatively, if
it is decided to have this lib available on all platforms, perhaps there
will be a configure option, in which case the provided patch would need to
reference some new flag, as opposed to "ifeq ($(PORTNAME), win32)".

Two other comments:

* timezone/private.h looks like it should be picking up its constants in
another way (ie. had to comment out HAVE_SYMLINK). Looks like we'll need to
spend some time in this file if we want it platform independent.

* pgwin32_TZDIR has the same hack as dfmgr.c:substitute_libpath_macro.
Please note that a "generalised" win32 solution to this was rejected in a
previous thread
(http://archives.postgresql.org/pgsql-patches/2004-03/msg00365.php), as it
was viewed as both restrictive (having to maintain a fixed PostgreSQL
subdirectory structure, which IMHO is not an onerous restriction for win32;
if anything it is expected by win32 users, and could simplify our installer)
and it was also suggested that a platform independent solution would be
preferable. However, to date I have seen no other solutions or suggestions.
As it is, we currently have the worst of both worlds, in that we have two
places in the code to maintain this kludge, a few more places that need it
before we can go beta, *and* it requires a fixed subdirectory structure.
Unless someone has any better ideas, I'm going to suggest resurrecting this
patch...

Cheers,
Claudio

--- 
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see 
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em
ailpolicy.html</a>

[ Attachment, skipping... ]

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.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 writes:

I am confused why you are linking the timezone library into
pg_dump and pg_controldata. Is that required?

At the time, cvs HEAD required it; perhaps I caught you mid-commit. I
thought it was a fast moving target to try to hit. :-)

All looks good now.

Cheers,
Claudio

--- 
Certain disclaimers and policies apply to all email sent from Memetrics.
For the full text of these disclaimers and policies see 
<a
href="http://www.memetrics.com/emailpolicy.html">http://www.memetrics.com/em
ailpolicy.html</a>

Peter Eisentraut <peter_e@gmx.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@gmx.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@candle.pha.pa.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@candle.pha.pa.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@candle.pha.pa.us> writes:

Tom Lane wrote:

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?

We will still need the timezone database files.

Good point, I had forgotten about the data/ subdirectory.

I think src/timezone is an okay place for it ...

regards, tom lane