Re: Australian timezone configure option

Hi,

Being in Australia, it's always been a minor pain building the support
for Australian timezone rules by defining USE_AUSTRALIAN_RULES to the
compiler. Not to mention the not inconsiderable pain involved in pawing
through the code and documentation trying to work out why the timezones
were wrong in the first place.

This patch makes it a configure option - much easier to use, and much
more obvious for the other Aussies who keep wondering why their
timezones are all messed up...

Obviously 'autoconf' needs to be run after applying the patch.

Cheers,

Chris,
OnTheNet

--- postgresql-7.1.2/configure.in.orig Fri May 11 11:34:39 2001
+++ postgresql-7.1.2/configure.in Thu May 31 23:54:27 2001
@@ -150,6 +150,16 @@


#
+# Australian timezone (--enable-australian-tz)
+#
+AC_MSG_CHECKING([whether to build with Australian timezone rules])
+PGAC_ARG_BOOL(enable, australian-tz, no, [ --enable-australian-tz enable Australian timezone rules ],
+ [AC_DEFINE([USE_AUSTRALIAN_RULES], 1,
+ [Set to 1 if you want Australian timezone rules (--enable-australian-tz)])])
+AC_MSG_RESULT([$enable_australian_tz])
+
+
+#
# Locale (--enable-locale)
#
AC_MSG_CHECKING([whether to build with locale support])

Chris Dunlop <chris(at)onthe(dot)net(dot)au> writes:
> This patch makes it a configure option - much easier to use,

Seems like a good idea, but that patch couldn't possibly work as-is.
Where's the config.h.in entry? Have you tested it?

regards, tom lane

On Mon, Jun 04, 2001 at 10:25:02AM -0400, Tom Lane wrote:
> Chris Dunlop <chris(at)onthe(dot)net(dot)au> writes:
> > This patch makes it a configure option - much easier to use,
>
> Seems like a good idea, but that patch couldn't possibly work as-is.
> Where's the config.h.in entry? Have you tested it?
>
> regards, tom lane

Oops... overzealeous trimming of the actual patch file I generated
which included all the changes to 'configure' generated by autoconf.

Patch including config.h.in changes below.

Test against the unpatched database, local timezone is "Australian EST"
i.e. GMT+10:

$ psql -c "select 'Jun 6 02:34:32 EST 2001'::datetime" template1
?column?
------------------------
2001-06-06 17:34:32+10
(1 row)

Notice the returned time is different to the input time. Against
the patched database:

psql -c "select 'Jun 6 02:34:32 EST 2001'::datetime" template1
?column?
------------------------
2001-06-06 02:34:32+10
(1 row)

Cheers,

Chris,
OnTheNet

diff -ru postgresql-7.1.2.orig/configure.in postgresql-7.1.2/configure.in
--- postgresql-7.1.2.orig/configure.in Fri May 11 11:34:39 2001
+++ postgresql-7.1.2/configure.in Thu May 31 23:54:27 2001
@@ -150,6 +150,16 @@


#
+# Australian timezone (--enable-australian-tz)
+#
+AC_MSG_CHECKING([whether to build with Australian timezone rules])
+PGAC_ARG_BOOL(enable, australian-tz, no, [ --enable-australian-tz enable Australian timezone rules ],
+ [AC_DEFINE([USE_AUSTRALIAN_RULES], 1,
+ [Set to 1 if you want Australian timezone rules (--enable-australian-tz)])])
+AC_MSG_RESULT([$enable_australian_tz])
+
+
+#
# Locale (--enable-locale)
#
AC_MSG_CHECKING([whether to build with locale support])
diff -ru postgresql-7.1.2.orig/src/include/config.h.in postgresql-7.1.2/src/include/config.h.in
--- postgresql-7.1.2.orig/src/include/config.h.in Sun Apr 15 08:55:02 2001
+++ postgresql-7.1.2/src/include/config.h.in Thu May 31 23:58:16 2001
@@ -33,6 +33,9 @@
/* A canonical string containing the version number, platform, and C compiler */
#undef PG_VERSION_STR

+/* Set to 1 if you want Australian timezone rules (--enable-australian-tz) */
+#undef USE_AUSTRALIAN_RULES
+
/* Set to 1 if you want LOCALE support (--enable-locale) */
#undef USE_LOCALE

Chris Dunlop <chris(at)onthe(dot)net(dot)au> writes:
> Oops... overzealeous trimming of the actual patch file I generated
> which included all the changes to 'configure' generated by autoconf.
> Patch including config.h.in changes below.

That looks better.

Could we also trouble you for documentation patches? IIRC, there's a
list of all interesting configure options somewhere in the
administrator's guide.

regards, tom lane

Tom Lane wrote:
> Could we also trouble you for documentation patches? IIRC, there's a
> list of all interesting configure options somewhere in the
> administrator's guide.

Documentation ? We don't need no steenking documentation!...

diff -ur postgresql-7.1.2.orig/doc/src/sgml/installation.sgml postgresql-7.1.2/doc/src/sgml/installation.sgml
--- postgresql-7.1.2.orig/doc/src/sgml/installation.sgml Tue May 15 01:11:31 2001
+++ postgresql-7.1.2/doc/src/sgml/installation.sgml Wed Jun 6 10:35:30 2001
@@ -462,6 +462,20 @@
</varlistentry>

<varlistentry>
+ <term>--enable-australian-tz</term>
+ <listitem>
+ <para>
+ Enables Australian timezone support. This changes the interpretation
+ of timezones in input date/time strings from US-centric to
+ Australian-centric. Specifically, 'EST' is changed from GMT-5 (US
+ Eastern Standard Time) to GMT+10 (Australian Eastern Standard Time)
+ and 'CST' is changed from GMT-5:30 (US Central Standard Time) to
+ GMT+10:30 (Australian Central Standard Time).
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term>--enable-locale</term>
<listitem>
<para>

I have decided to make this configurable via postgresql.conf so you
don't need separate binaries / configure switch to run in Australia. I
will send a patch over for testing.

> Tom Lane wrote:
> > Could we also trouble you for documentation patches? IIRC, there's a
> > list of all interesting configure options somewhere in the
> > administrator's guide.
>
>
> Documentation ? We don't need no steenking documentation!...
>
>
> diff -ur postgresql-7.1.2.orig/doc/src/sgml/installation.sgml postgresql-7.1.2/doc/src/sgml/installation.sgml
> --- postgresql-7.1.2.orig/doc/src/sgml/installation.sgml Tue May 15 01:11:31 2001
> +++ postgresql-7.1.2/doc/src/sgml/installation.sgml Wed Jun 6 10:35:30 2001
> @@ -462,6 +462,20 @@
> </varlistentry>
>
> <varlistentry>
> + <term>--enable-australian-tz</term>
> + <listitem>
> + <para>
> + Enables Australian timezone support. This changes the interpretation
> + of timezones in input date/time strings from US-centric to
> + Australian-centric. Specifically, 'EST' is changed from GMT-5 (US
> + Eastern Standard Time) to GMT+10 (Australian Eastern Standard Time)
> + and 'CST' is changed from GMT-5:30 (US Central Standard Time) to
> + GMT+10:30 (Australian Central Standard Time).
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> <term>--enable-locale</term>
> <listitem>
> <para>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Hi all,

Can we *not* make USE_AUSTRALIAN_RULES into a "configure" option?

Seems like this would conflict with Tom's suggestion of making it a GUC
option instead.

I vote for having it as a GUC option instead.

Regards and best wishes,

Justin Clift

On Wednesday 06 June 2001 10:43, Chris Dunlop wrote:
> Tom Lane wrote:
> > Could we also trouble you for documentation patches? IIRC, there's a
> > list of all interesting configure options somewhere in the
> > administrator's guide.
>
> Documentation ? We don't need no steenking documentation!...
>
>
> diff -ur postgresql-7.1.2.orig/doc/src/sgml/installation.sgml
> postgresql-7.1.2/doc/src/sgml/installation.sgml ---
> postgresql-7.1.2.orig/doc/src/sgml/installation.sgml Tue May 15 01:11:31
> 2001 +++ postgresql-7.1.2/doc/src/sgml/installation.sgml Wed Jun 6
> 10:35:30 2001 @@ -462,6 +462,20 @@
> </varlistentry>
>
> <varlistentry>
> + <term>--enable-australian-tz</term>
> + <listitem>
> + <para>
> + Enables Australian timezone support. This changes the
> interpretation + of timezones in input date/time strings from
> US-centric to + Australian-centric. Specifically, 'EST' is changed
> from GMT-5 (US + Eastern Standard Time) to GMT+10 (Australian
> Eastern Standard Time) + and 'CST' is changed from GMT-5:30 (US
> Central Standard Time) to + GMT+10:30 (Australian Central Standard
> Time).
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> <term>--enable-locale</term>
> <listitem>
> <para>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html

On Mon, Jun 11, 2001 at 06:52:52PM -0400, Bruce Momjian wrote:
>
> I have decided to make this configurable via postgresql.conf so you
> don't need separate binaries / configure switch to run in Australia. I
> will send a patch over for testing.

Great, that's better than a configure option...

> > diff -ur postgresql-7.1.2.orig/doc/src/sgml/installation.sgml postgresql-7.1.2/doc/src/sgml/installation.sgml
> > --- postgresql-7.1.2.orig/doc/src/sgml/installation.sgml Tue May 15 01:11:31 2001
> > +++ postgresql-7.1.2/doc/src/sgml/installation.sgml Wed Jun 6 10:35:30 2001
> > @@ -462,6 +462,20 @@
> > </varlistentry>
> >
> > <varlistentry>
> > + <term>--enable-australian-tz</term>
> > + <listitem>
> > + <para>
> > + Enables Australian timezone support. This changes the interpretation
> > + of timezones in input date/time strings from US-centric to
> > + Australian-centric. Specifically, 'EST' is changed from GMT-5 (US
> > + Eastern Standard Time) to GMT+10 (Australian Eastern Standard Time)
> > + and 'CST' is changed from GMT-5:30 (US Central Standard Time) to
> > + GMT+10:30 (Australian Central Standard Time).
> > + </para>
> > + </listitem>
> > + </varlistentry>
> > +
> > + <varlistentry>
> > <term>--enable-locale</term>
> > <listitem>
> > <para>

On Tue, Jun 12, 2001 at 12:25:11PM +1000, Justin Clift wrote:
> Hi all,
>
> Can we *not* make USE_AUSTRALIAN_RULES into a "configure" option?
>
> Seems like this would conflict with Tom's suggestion of making it a GUC
> option instead.
>
> I vote for having it as a GUC option instead.

Yup, I think Bruce is actually making it a GUC option.

>
> Regards and best wishes,
>
> Justin Clift
>

> On Mon, Jun 11, 2001 at 06:52:52PM -0400, Bruce Momjian wrote:
> >
> > I have decided to make this configurable via postgresql.conf so you
> > don't need separate binaries / configure switch to run in Australia. I
> > will send a patch over for testing.
>
>
> Great, that's better than a configure option...
>

I thought so.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> Hi,
>
> Being in Australia, it's always been a minor pain building the support
> for Australian timezone rules by defining USE_AUSTRALIAN_RULES to the
> compiler. Not to mention the not inconsiderable pain involved in pawing
> through the code and documentation trying to work out why the timezones
> were wrong in the first place.

OK, this patch makes Australian_timezones a GUC option. It can be set
anytime in psql. The code uses a static variable to check if the GUC
setting has changed and adjust the C struct accordingly. I have also
added code to allow the regression tests to pass even if postgresql.conf
has australian_timezones defined.

test=> select datetime('2001-01-01 00:00:00 EST');
timestamp
------------------------
2001-01-01 00:00:00-05
(1 row)

test=> set australian_timezones = true;
SET VARIABLE
test=> select datetime('2001-01-01 00:00:00 EST');
timestamp
------------------------
2000-12-31 09:00:00-05
(1 row)
test=> reset all;
RESET VARIABLE
test=> select datetime('2001-01-01 00:00:00 EST');
timestamp
------------------------
2001-01-01 00:00:00-05
(1 row)

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> > Hi,
> >
> > Being in Australia, it's always been a minor pain building the support
> > for Australian timezone rules by defining USE_AUSTRALIAN_RULES to the
> > compiler. Not to mention the not inconsiderable pain involved in pawing
> > through the code and documentation trying to work out why the timezones
> > were wrong in the first place.
>
> OK, this patch makes Australian_timezones a GUC option. It can be set
> anytime in psql. The code uses a static variable to check if the GUC
> setting has changed and adjust the C struct accordingly. I have also
> added code to allow the regression tests to pass even if postgresql.conf
> has australian_timezones defined.

Here is a diff for the documentation.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

On Mon, Jun 11, 2001 at 11:53:59PM -0400, Bruce Momjian wrote:
> > Hi,
> >
> > Being in Australia, it's always been a minor pain building the support
> > for Australian timezone rules by defining USE_AUSTRALIAN_RULES to the
> > compiler. Not to mention the not inconsiderable pain involved in pawing
> > through the code and documentation trying to work out why the timezones
> > were wrong in the first place.
>
> OK, this patch makes Australian_timezones a GUC option. It can be set
> anytime in psql. The code uses a static variable to check if the GUC
> setting has changed and adjust the C struct accordingly. I have also
> added code to allow the regression tests to pass even if postgresql.conf
> has australian_timezones defined.

Your patch had one reject against 7.1.2 (a single blank line in guc.c),
but it works for me once that was fixed.

Below is the patch against 7.1.2 I generated from your cvs patch.

I guess some documentation would be nice...

Thanks for your effort!

Cheers,

Chris,
OnTheNet

diff -ru postgresql-7.1.2.orig/src/backend/utils/adt/datetime.c postgresql-7.1.2/src/backend/utils/adt/datetime.c
--- postgresql-7.1.2.orig/src/backend/utils/adt/datetime.c Fri May 4 08:53:07 2001
+++ postgresql-7.1.2/src/backend/utils/adt/datetime.c Tue Jun 12 14:14:38 2001
@@ -22,6 +22,7 @@
#include <limits.h>

#include "miscadmin.h"
+#include "utils/guc.h"
#include "utils/datetime.h"

static int DecodeNumber(int flen, char *field,
@@ -35,6 +36,7 @@
static int DecodeTimezone(char *str, int *tzp);
static datetkn *datebsearch(char *key, datetkn *base, unsigned int nel);
static int DecodeDate(char *str, int fmask, int *tmask, struct tm * tm);
+static void CheckAustralianTimezones(int field);

#define USE_DATE_CACHE 1
#define ROUND_ALL 0
@@ -85,7 +87,7 @@
* entries by 10 and truncate the text field at MAXTOKLEN characters.
* the text field is not guaranteed to be NULL-terminated.
*/
-static datetkn datetktbl[] = {
+datetkn datetktbl[] = {
/* text token lexval */
{EARLY, RESERV, DTK_EARLY}, /* "-infinity" reserved for "early time" */
{"acsst", DTZ, 63}, /* Cent. Australia */
@@ -117,11 +119,7 @@
{"cdt", DTZ, NEG(30)}, /* Central Daylight Time */
{"cet", TZ, 6}, /* Central European Time */
{"cetdst", DTZ, 12}, /* Central European Dayl.Time */
-#if USE_AUSTRALIAN_RULES
- {"cst", TZ, 63}, /* Australia Eastern Std Time */
-#else
- {"cst", TZ, NEG(36)}, /* Central Standard Time */
-#endif
+ {"cst", TZ, NEG(36)}, /* Central Standard Time, may be Australian */
{DCURRENT, RESERV, DTK_CURRENT}, /* "current" is always now */
{"dec", MONTH, 12},
{"december", MONTH, 12},
@@ -134,11 +132,7 @@
{"eet", TZ, 12}, /* East. Europe, USSR Zone 1 */
{"eetdst", DTZ, 18}, /* Eastern Europe */
{EPOCH, RESERV, DTK_EPOCH}, /* "epoch" reserved for system epoch time */
-#if USE_AUSTRALIAN_RULES
- {"est", TZ, 60}, /* Australia Eastern Std Time */
-#else
- {"est", TZ, NEG(30)}, /* Eastern Standard Time */
-#endif
+ {"est", TZ, NEG(30)}, /* Eastern Standard Time, may be Australian */
{"feb", MONTH, 2},
{"february", MONTH, 2},
{"fri", DOW, 5},
@@ -199,11 +193,7 @@
{"pst", TZ, NEG(48)}, /* Pacific Standard Time */
{"sadt", DTZ, 63}, /* S. Australian Dayl. Time */
{"sast", TZ, 57}, /* South Australian Std Time */
-#if USE_AUSTRALIAN_RULES
- {"sat", TZ, 57},
-#else
- {"sat", DOW, 6},
-#endif
+ {"sat", DOW, 6}, /* may be changed to Australian */
{"saturday", DOW, 6},
{"sep", MONTH, 9},
{"sept", MONTH, 9},
@@ -1618,6 +1608,8 @@
int type;
datetkn *tp;

+ CheckAustralianTimezones(field);
+
#if USE_DATE_CACHE
if ((datecache[field] != NULL)
&& (strncmp(lowtoken, datecache[field]->token, TOKMAXLEN) == 0))
@@ -2455,3 +2447,49 @@

return 0;
} /* EncodeTimeSpan() */
+
+
+static void CheckAustralianTimezones(int field)
+{
+ datetkn *tp;
+ int prev_Australian_timezones = false; /* structure preloaded as false */
+
+ if (Australian_timezones != prev_Australian_timezones)
+ {
+#if USE_DATE_CACHE
+ datecache[field] = NULL;
+#endif
+ /* CST */
+ tp = datebsearch("cst", datetktbl, szdatetktbl);
+ Assert(tp);
+ tp->type = TZ;
+ if (!Australian_timezones)
+ tp->value = NEG(36); /* Central Standard Time */
+ else
+ tp->value = 63; /* Australia Eastern Std Time */
+
+ /* EST */
+ tp = datebsearch("est", datetktbl, szdatetktbl);
+ Assert(tp);
+ tp->type = TZ;
+ if (!Australian_timezones)
+ tp->value = NEG(30); /* Eastern Standard Time */
+ else
+ tp->value = 60; /* Australia Eastern Std Time */
+
+ /* SAT */
+ tp = datebsearch("sat", datetktbl, szdatetktbl);
+ Assert(tp);
+ if (!Australian_timezones)
+ {
+ tp->type = DOW;
+ tp->value = 6;
+ }
+ else
+ {
+ tp->type = TZ;
+ tp->value = 57;
+ }
+ prev_Australian_timezones = Australian_timezones;
+ }
+}
diff -ru postgresql-7.1.2.orig/src/backend/utils/misc/guc.c postgresql-7.1.2/src/backend/utils/misc/guc.c
--- postgresql-7.1.2.orig/src/backend/utils/misc/guc.c Fri Mar 23 04:41:47 2001
+++ postgresql-7.1.2/src/backend/utils/misc/guc.c Tue Jun 12 14:14:38 2001
@@ -70,6 +70,8 @@

bool SQL_inheritance = true;

+bool Australian_timezones = false;
+
#ifndef PG_KRB_SRVTAB
#define PG_KRB_SRVTAB ""
#endif
@@ -220,6 +222,7 @@
{"show_source_port", PGC_SIGHUP, &ShowPortNumber, false},

{"sql_inheritance", PGC_USERSET, &SQL_inheritance, true},
+ {"australian_timezones", PGC_USERSET, &Australian_timezones, false},

{"fixbtree", PGC_POSTMASTER, &FixBTree, true},

@@ -856,7 +859,7 @@
elog(FATAL, "out of memory");
}
else
-/* no equal sign in string */
+ /* no equal sign in string */
{
*name = strdup(string);
if (!*name)
diff -ru postgresql-7.1.2.orig/src/backend/utils/misc/postgresql.conf.sample postgresql-7.1.2/src/backend/utils/misc/postgresql.conf.sample
--- postgresql-7.1.2.orig/src/backend/utils/misc/postgresql.conf.sample Fri Mar 16 16:44:33 2001
+++ postgresql-7.1.2/src/backend/utils/misc/postgresql.conf.sample Tue Jun 12 14:14:38 2001
@@ -172,3 +172,9 @@
#trace_lock_oidmin = 16384
#trace_lock_table = 0
#endif
+
+
+#
+# Lock Tracing
+#
+#australian_timezones = false
diff -ru postgresql-7.1.2.orig/src/include/utils/datetime.h postgresql-7.1.2/src/include/utils/datetime.h
--- postgresql-7.1.2.orig/src/include/utils/datetime.h Fri May 4 08:53:07 2001
+++ postgresql-7.1.2/src/include/utils/datetime.h Tue Jun 12 14:14:38 2001
@@ -182,6 +182,7 @@
char value; /* this may be unsigned, alas */
} datetkn;

+extern datetkn datetktbl[];

/* TMODULO()
* Macro to replace modf(), which is broken on some platforms.
diff -ru postgresql-7.1.2.orig/src/include/utils/guc.h postgresql-7.1.2/src/include/utils/guc.h
--- postgresql-7.1.2.orig/src/include/utils/guc.h Thu Mar 22 15:01:12 2001
+++ postgresql-7.1.2/src/include/utils/guc.h Tue Jun 12 14:14:38 2001
@@ -67,5 +67,6 @@
extern bool Show_btree_build_stats;

extern bool SQL_inheritance;
+extern bool Australian_timezones;

#endif /* GUC_H */
diff -ru postgresql-7.1.2.orig/src/test/regress/expected/horology-no-DST-before-1970.out postgresql-7.1.2/src/test/regress/expected/horology-no-DST-before-1970.out
--- postgresql-7.1.2.orig/src/test/regress/expected/horology-no-DST-before-1970.out Fri Apr 6 15:50:25 2001
+++ postgresql-7.1.2/src/test/regress/expected/horology-no-DST-before-1970.out Tue Jun 12 14:14:38 2001
@@ -4,6 +4,8 @@
--
-- date, time arithmetic
--
+-- needed so tests pass
+SET australian_timezones = 'off';
SELECT date '1981-02-03' + time '04:05:06' AS "Date + Time";
Date + Time
------------------------------
diff -ru postgresql-7.1.2.orig/src/test/regress/expected/horology-solaris-1947.out postgresql-7.1.2/src/test/regress/expected/horology-solaris-1947.out
--- postgresql-7.1.2.orig/src/test/regress/expected/horology-solaris-1947.out Fri Apr 6 15:50:25 2001
+++ postgresql-7.1.2/src/test/regress/expected/horology-solaris-1947.out Tue Jun 12 14:14:38 2001
@@ -4,6 +4,8 @@
--
-- date, time arithmetic
--
+-- needed so tests pass
+SET australian_timezones = 'off';
SELECT date '1981-02-03' + time '04:05:06' AS "Date + Time";
Date + Time
------------------------------
diff -ru postgresql-7.1.2.orig/src/test/regress/expected/horology.out postgresql-7.1.2/src/test/regress/expected/horology.out
--- postgresql-7.1.2.orig/src/test/regress/expected/horology.out Fri Apr 6 15:50:25 2001
+++ postgresql-7.1.2/src/test/regress/expected/horology.out Tue Jun 12 14:14:38 2001
@@ -4,6 +4,8 @@
--
-- date, time arithmetic
--
+-- needed so tests pass
+SET australian_timezones = 'off';
SELECT date '1981-02-03' + time '04:05:06' AS "Date + Time";
Date + Time
------------------------------
diff -ru postgresql-7.1.2.orig/src/test/regress/expected/timestamp.out postgresql-7.1.2/src/test/regress/expected/timestamp.out
--- postgresql-7.1.2.orig/src/test/regress/expected/timestamp.out Fri May 4 05:00:37 2001
+++ postgresql-7.1.2/src/test/regress/expected/timestamp.out Tue Jun 12 14:14:38 2001
@@ -4,6 +4,8 @@
-- Shorthand values
-- Not directly usable for regression testing since these are not constants.
-- So, just try to test parser and hope for the best - thomas 97/04/26
+-- needed so tests pass
+SET australian_timezones = 'off';
SELECT (timestamp 'today' = (timestamp 'yesterday' + interval '1 day')) as "True";
True
------
diff -ru postgresql-7.1.2.orig/src/test/regress/sql/horology.sql postgresql-7.1.2/src/test/regress/sql/horology.sql
--- postgresql-7.1.2.orig/src/test/regress/sql/horology.sql Fri Apr 6 15:50:29 2001
+++ postgresql-7.1.2/src/test/regress/sql/horology.sql Tue Jun 12 14:14:38 2001
@@ -1,10 +1,11 @@
--
-- HOROLOGY
--
-
--
-- date, time arithmetic
--
+-- needed so tests pass
+SET australian_timezones = 'off';

SELECT date '1981-02-03' + time '04:05:06' AS "Date + Time";

diff -ru postgresql-7.1.2.orig/src/test/regress/sql/timestamp.sql postgresql-7.1.2/src/test/regress/sql/timestamp.sql
--- postgresql-7.1.2.orig/src/test/regress/sql/timestamp.sql Sat Nov 25 16:00:33 2000
+++ postgresql-7.1.2/src/test/regress/sql/timestamp.sql Tue Jun 12 14:14:38 2001
@@ -1,10 +1,11 @@
--
-- DATETIME
--
-
-- Shorthand values
-- Not directly usable for regression testing since these are not constants.
-- So, just try to test parser and hope for the best - thomas 97/04/26
+-- needed so tests pass
+SET australian_timezones = 'off';

SELECT (timestamp 'today' = (timestamp 'yesterday' + interval '1 day')) as "True";
SELECT (timestamp 'today' = (timestamp 'tomorrow' - interval '1 day')) as "True";

> > OK, this patch makes Australian_timezones a GUC option. It can be set
> > anytime in psql. The code uses a static variable to check if the GUC
> > setting has changed and adjust the C struct accordingly. I have also
> > added code to allow the regression tests to pass even if postgresql.conf
> > has australian_timezones defined.
>
>
> Your patch had one reject against 7.1.2 (a single blank line in guc.c),
> but it works for me once that was fixed.
>
> Below is the patch against 7.1.2 I generated from your cvs patch.

We will not be adding this to 7.1.X though you are free to use it in
that version. It will appear in 7.2.

> I guess some documentation would be nice...

Done.

Thanks for testing it.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> OK, this patch makes Australian_timezones a GUC option. It can be set
> anytime in psql. The code uses a static variable to check if the GUC
> setting has changed and adjust the C struct accordingly.

This is a horrid approach. What if you get an error partway through
scribbling on the static table? Then you've got inconsistent data.

Nor do I much care for having to execute a check subroutine before any
use of the lookup table (quite aside from speed, are you sure it's being
called before *every* use of the table? how will you make sure that
people remember to call it when they add new routines that use the
table?). If you're going to scribble on the table, ISTM you should
drive that off an assignment-hook callback from the GUC stuff.

Besides which, you forgot to mark the control variable static...
so it doesn't actually reflect the state of the table.

It would be a lot cleaner to extend the lookup table structure so that
you don't need to change the table contents to track the Aussie-rules
setting.

regards, tom lane

(moved to -hackers list, since this is a *feature change* which should
not just be discussed in -patches)

>>I have decided to make this configurable via postgresql.conf so you
>>don't need separate binaries / configure switch to run in Australia.
>>I will send a patch over for testing.
> We will not be adding this to 7.1.X though you are free to use it in
> that version. It will appear in 7.2.

Um, er...

I'm not particularly happy about the solution, and would like to not see
it in the main source code without further discussion. Sorry I was out
of town for the extensive two hour discussion on the topic ;)

One could categorize the "Australian problem" as an example of a
localization, and brute-force calls to work around it are a step in the
wrong direction imho. Particularly since Australia contributes fully 25%
of the time zone information in our lookup table, including multiple
synonyms for several time zones. Just as we might want to send a message
to M$ that their SQL hacks are not acceptable, we should send a message
to Australia that playing fast and loose with time zone names should not
be tolerated. Hmm, and while we are at it we should do something about
world hunger and arms race issues. Patches coming soon ;) ;)

OK, those last few sentences were not serious. But, I would like a
solution that starts to address long-term issues in time zone support.

Before hacking the rather carefully evolved static tables let's consider
how to support time localization generally (e.g. language-specific names
for months). In the meantime, a compile-time solution for more easily
setting the "CST" interpretation would seem to be an incremental
improvement for "buildability" (and this has already been submitted).

How about implementing an optional db table-based approach for this
lookup and the other localized info? If it were a compile-time option we
could evaluate the performance impact and allow folks to trade
performance vs features. And perhaps (later, much later. Weeks later...
;) that choice could be a GUC parameter. Comments?

- Thomas

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > OK, this patch makes Australian_timezones a GUC option. It can be set
> > anytime in psql. The code uses a static variable to check if the GUC
> > setting has changed and adjust the C struct accordingly.
>
> This is a horrid approach. What if you get an error partway through
> scribbling on the static table? Then you've got inconsistent data.

I have set the variable to -1 on entry so it will reload on failure,
though it is only doing C lookups on a static table so it is hard to see
how it would fail.

> Nor do I much care for having to execute a check subroutine before any
> use of the lookup table (quite aside from speed, are you sure it's being
> called before *every* use of the table? how will you make sure that
> people remember to call it when they add new routines that use the
> table?). If you're going to scribble on the table, ISTM you should
> drive that off an assignment-hook callback from the GUC stuff.

But we don't have such hooks so I did it with as little code as
possible. The table itself is 'static' so it is only called in this
function. (The old patch had the static removed because I thought I was
going to have to do this stuff in the guc files but now it is all in the
same file.)

> Besides which, you forgot to mark the control variable static...
> so it doesn't actually reflect the state of the table.

Fixed. I had that in an ealier version but forgot to add it when I
moved it out to a separate function.

> It would be a lot cleaner to extend the lookup table structure so that
> you don't need to change the table contents to track the Aussie-rules
> setting.

Yes, when we do that we can remove the call and call it instead from the
GUC setting.

Here is a new version of the patch. I found I didn't need to clear the
date cache.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> (moved to -hackers list, since this is a *feature change* which should
> not just be discussed in -patches)
>
> >>I have decided to make this configurable via postgresql.conf so you
> >>don't need separate binaries / configure switch to run in Australia.
> >>I will send a patch over for testing.
> > We will not be adding this to 7.1.X though you are free to use it in
> > that version. It will appear in 7.2.
>
> Um, er...
>
> I'm not particularly happy about the solution, and would like to not see
> it in the main source code without further discussion. Sorry I was out
> of town for the extensive two hour discussion on the topic ;)
>
> One could categorize the "Australian problem" as an example of a
> localization, and brute-force calls to work around it are a step in the
> wrong direction imho. Particularly since Australia contributes fully 25%
> of the time zone information in our lookup table, including multiple
> synonyms for several time zones. Just as we might want to send a message
> to M$ that their SQL hacks are not acceptable, we should send a message
> to Australia that playing fast and loose with time zone names should not
> be tolerated. Hmm, and while we are at it we should do something about
> world hunger and arms race issues. Patches coming soon ;) ;)
>
> OK, those last few sentences were not serious. But, I would like a
> solution that starts to address long-term issues in time zone support.
>
> Before hacking the rather carefully evolved static tables let's consider
> how to support time localization generally (e.g. language-specific names
> for months). In the meantime, a compile-time solution for more easily
> setting the "CST" interpretation would seem to be an incremental
> improvement for "buildability" (and this has already been submitted).
>
> How about implementing an optional db table-based approach for this
> lookup and the other localized info? If it were a compile-time option we
> could evaluate the performance impact and allow folks to trade
> performance vs features. And perhaps (later, much later. Weeks later...
> ;) that choice could be a GUC parameter. Comments?

This is way beyond where I want to go with the Australian stuff and I
have not seen much demand from users for more than a GUC option.
Australians wanted a 'configure' flag, I made it GUC which gets reloaded
on first call and can later be assigned to a GUC hook when we get those.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Thomas Lockhart writes:

> Before hacking the rather carefully evolved static tables let's consider
> how to support time localization generally (e.g. language-specific names
> for months). In the meantime, a compile-time solution for more easily
> setting the "CST" interpretation would seem to be an incremental
> improvement for "buildability" (and this has already been submitted).

I'm not particularly happy about "popularizing" that compile time option
beyond its current state (i.e., get in and edit config.h), and there's a
reason why I haven't done it myself yet.

--enable-xxx type configure options should, as a matter of principle, not
replace one behaviour by another. (The proposed option replaces U.S.
rules by Australian rules.) In this case it might look like a minor
issue, but it's a slippery slope. For one thing, packages build by
Australians will cease to behave reasonably in the rest of the world.
That is the same reason why we don't want people altering NAMEDATALEN and
BLCKSZ from configure.

A run-time option seems like the appropriate solution.

--
Peter Eisentraut peter_e(at)gmx(dot)net http://funkturm.homeip.net/~peter

> Thomas Lockhart writes:
>
> > Before hacking the rather carefully evolved static tables let's consider
> > how to support time localization generally (e.g. language-specific names
> > for months). In the meantime, a compile-time solution for more easily
> > setting the "CST" interpretation would seem to be an incremental
> > improvement for "buildability" (and this has already been submitted).
>
> I'm not particularly happy about "popularizing" that compile time option
> beyond its current state (i.e., get in and edit config.h), and there's a
> reason why I haven't done it myself yet.
>
> --enable-xxx type configure options should, as a matter of principle, not
> replace one behaviour by another. (The proposed option replaces U.S.
> rules by Australian rules.) In this case it might look like a minor
> issue, but it's a slippery slope. For one thing, packages build by
> Australians will cease to behave reasonably in the rest of the world.
> That is the same reason why we don't want people altering NAMEDATALEN and
> BLCKSZ from configure.
>
> A run-time option seems like the appropriate solution.

Agreed. Compile-time is just the wrong way to go. We should have
compile-time stuff that just relates to the OS/compiler and sometimes
installed software, or simply stuff that just can't be changed without a
recompile/initdb like NAMEDATALEN.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Here is a new version of the patch.

It's still horridly ugly. Why not put the three Aussie-specific entries
in a separate aussie_datetktbl array, and have the lookup look like

if (Australian_timezones)
{
tp = datebsearch(lowtoken, aussie_datetktbl, sz_aussie_datetktbl);
if (tp == NULL)
tp = datebsearch(lowtoken, datetktbl, szdatetktbl);
}
else
tp = datebsearch(lowtoken, datetktbl, szdatetktbl);

instead of modifying the lookup table on the fly.

> I found I didn't need to clear the date cache.

Hmm, are you sure about that? I'm not.

regards, tom lane

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Here is a new version of the patch.
>
> It's still horridly ugly. Why not put the three Aussie-specific entries
> in a separate aussie_datetktbl array, and have the lookup look like
>
> if (Australian_timezones)
> {
> tp = datebsearch(lowtoken, aussie_datetktbl, sz_aussie_datetktbl);
> if (tp == NULL)
> tp = datebsearch(lowtoken, datetktbl, szdatetktbl);
> }
> else
> tp = datebsearch(lowtoken, datetktbl, szdatetktbl);
>
> instead of modifying the lookup table on the fly.

I thought about that but the use of the cache had me concerned,
particularly for modularity. This way there is only one table to query
with one call to the bsearch function.

However, I can go with the two-table approach if people prefer it.

> > I found I didn't need to clear the date cache.
>
> Hmm, are you sure about that? I'm not.

I checked and it caches a pointer to the struct, not the values
themselves, and we don't change the structure, just the secondary values
and not the key used by the bsearch.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> I found I didn't need to clear the date cache.
>>
>> Hmm, are you sure about that? I'm not.

> I checked and it caches a pointer to the struct, not the values
> themselves, and we don't change the structure, just the secondary values
> and not the key used by the bsearch.

Now I'm going to object LOUDLY. You cannot convince me that the above
is a good implementation --- it's a complete crock, and will break the
instant someone looks at it sidewise.

My inclination would actually be to rip out the cache entirely. bsearch
in a table this size is not so expensive that we need to bypass it, nor
is it apparent that we are going to see lots of successive lookups for
the same keyword anyway. How long has that cache been in there, and
what was the motivation for adding it to begin with?

regards, tom lane

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > I found I didn't need to clear the date cache.
> >>
> >> Hmm, are you sure about that? I'm not.
>
> > I checked and it caches a pointer to the struct, not the values
> > themselves, and we don't change the structure, just the secondary values
> > and not the key used by the bsearch.
>
> Now I'm going to object LOUDLY. You cannot convince me that the above
> is a good implementation --- it's a complete crock, and will break the
> instant someone looks at it sidewise.
>
> My inclination would actually be to rip out the cache entirely. bsearch
> in a table this size is not so expensive that we need to bypass it, nor
> is it apparent that we are going to see lots of successive lookups for
> the same keyword anyway. How long has that cache been in there, and
> what was the motivation for adding it to begin with?

I see the CACHE coming in with:

1.42 (thomas 16-Feb-00): #define USE_DATE_CACHE 1

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> > I checked and it caches a pointer to the struct, not the values
> > themselves, and we don't change the structure, just the secondary values
> > and not the key used by the bsearch.
>
> Now I'm going to object LOUDLY. You cannot convince me that the above
> is a good implementation --- it's a complete crock, and will break the
> instant someone looks at it sidewise.
>
> My inclination would actually be to rip out the cache entirely. bsearch
> in a table this size is not so expensive that we need to bypass it, nor
> is it apparent that we are going to see lots of successive lookups for
> the same keyword anyway. How long has that cache been in there, and
> what was the motivation for adding it to begin with?

OK, what do we do with this patch now? We have several Australians who
want it and most like it as GUC rather than a configure option. I don't
want to add lots more code because I think the GUC capability is simple
enough. I can remove the CACHE stuff but only if Thomas agrees.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> <varlistentry>
> + <term>--enable-australian-tz</term>
> + <listitem>
> + <para>
> + Enables Australian timezone support. This changes the
> interpretation
> + of timezones in input date/time strings from US-centric to
> + Australian-centric. Specifically, 'EST' is changed
> from GMT-5 (US
> + Eastern Standard Time) to GMT+10 (Australian Eastern
> Standard Time)
> + and 'CST' is changed from GMT-5:30 (US Central Standard Time) to
> + GMT+10:30 (Australian Central Standard Time).
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> <term>--enable-locale</term>
> <listitem>
> <para>

What about us West Australians living in WST (Western Standard Time) ?

Chris

"Christopher Kings-Lynne" <chriskl(at)familyhealth(dot)com(dot)au> writes:
> What about us West Australians living in WST (Western Standard Time) ?

You don't have a problem.

It's only the TZ abbreviations that conflict with US usages that need
to be driven by a switch. The rest of them are just there...

regards, tom lane

>
> > <varlistentry>
> > + <term>--enable-australian-tz</term>
> > + <listitem>
> > + <para>
> > + Enables Australian timezone support. This changes the
> > interpretation
> > + of timezones in input date/time strings from US-centric to
> > + Australian-centric. Specifically, 'EST' is changed
> > from GMT-5 (US
> > + Eastern Standard Time) to GMT+10 (Australian Eastern
> > Standard Time)
> > + and 'CST' is changed from GMT-5:30 (US Central Standard Time) to
> > + GMT+10:30 (Australian Central Standard Time).
> > + </para>
> > + </listitem>
> > + </varlistentry>
> > +
> > + <varlistentry>
> > <term>--enable-locale</term>
> > <listitem>
> > <para>
>
> What about us West Australians living in WST (Western Standard Time) ?

That is already in there. The setting is for names that conflict with
other names:

{"wst", TZ, 48}, /* West Australian Std Time */

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> On Mon, Jun 11, 2001 at 11:53:59PM -0400, Bruce Momjian wrote:
> > > Hi,
> > >
> > > Being in Australia, it's always been a minor pain building the support
> > > for Australian timezone rules by defining USE_AUSTRALIAN_RULES to the
> > > compiler. Not to mention the not inconsiderable pain involved in pawing
> > > through the code and documentation trying to work out why the timezones
> > > were wrong in the first place.
> >
> > OK, this patch makes Australian_timezones a GUC option. It can be set
> > anytime in psql. The code uses a static variable to check if the GUC
> > setting has changed and adjust the C struct accordingly. I have also
> > added code to allow the regression tests to pass even if postgresql.conf
> > has australian_timezones defined.

Here is a new version of the patch. Tom added callbacks to GUC boolean
variables so I was able to make a separate Australian lookup table and
clear the cache if anyone changes the setting.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> Now I'm going to object LOUDLY. You cannot convince me that the above
> is a good implementation --- it's a complete crock, and will break the
> instant someone looks at it sidewise.

But it hasn't broken in years of use and maintenance, so that does not
sound like an issue. Eventually, we may want full localization with
character sets *and* date/time conventions, and all of this can be
rethought at that time.

> My inclination would actually be to rip out the cache entirely. bsearch
> in a table this size is not so expensive that we need to bypass it, nor
> is it apparent that we are going to see lots of successive lookups for
> the same keyword anyway. How long has that cache been in there, and
> what was the motivation for adding it to begin with?

The cache lookup came from the gods themselves at Berkeley ;)

- Thomas

On Wed, Jun 13, 2001 at 01:16:51PM -0400, Bruce Momjian wrote:
> > On Mon, Jun 11, 2001 at 11:53:59PM -0400, Bruce Momjian wrote:
> > > > Hi,
> > > >
> > > > Being in Australia, it's always been a minor pain building the support
> > > > for Australian timezone rules by defining USE_AUSTRALIAN_RULES to the
> > > > compiler. Not to mention the not inconsiderable pain involved in pawing
> > > > through the code and documentation trying to work out why the timezones
> > > > were wrong in the first place.
> > >
> > > OK, this patch makes Australian_timezones a GUC option. It can be set
> > > anytime in psql. The code uses a static variable to check if the GUC
> > > setting has changed and adjust the C struct accordingly. I have also
> > > added code to allow the regression tests to pass even if postgresql.conf
> > > has australian_timezones defined.
>
> Here is a new version of the patch. Tom added callbacks to GUC boolean
> variables so I was able to make a separate Australian lookup table and
> clear the cache if anyone changes the setting.

With the new Australian timezone patch requiring Tom's callback patch
we're diverging more and more from the standard 7.1.2 code base, so for
my own 7.1.2 systems I'm going to revert to my original 'configure'
patch.

Thanks for your efforts on this (and everything else of course), and
looking forward to the 7.2 code!

Cheers,

Chris,
OnTheNet

> > My inclination would actually be to rip out the cache entirely. bsearch
> > in a table this size is not so expensive that we need to bypass it, nor
> > is it apparent that we are going to see lots of successive lookups for
> > the same keyword anyway. How long has that cache been in there, and
> > what was the motivation for adding it to begin with?
>
> The cache lookup came from the gods themselves at Berkeley ;)

Wow, I didn't know that. I like the phrase.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

> > Here is a new version of the patch. Tom added callbacks to GUC boolean
> > variables so I was able to make a separate Australian lookup table and
> > clear the cache if anyone changes the setting.
>
> With the new Australian timezone patch requiring Tom's callback patch
> we're diverging more and more from the standard 7.1.2 code base, so for
> my own 7.1.2 systems I'm going to revert to my original 'configure'
> patch.
>
> Thanks for your efforts on this (and everything else of course), and
> looking forward to the 7.2 code!

Yes, I was concerned about that. If you need GUC, the second patch I
submitted with fix for static and cache lookup should be fine in 7.1.X.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Thomas Lockhart <lockhart(at)alumni(dot)caltech(dot)edu> writes:
>> Now I'm going to object LOUDLY. You cannot convince me that the above
>> is a good implementation --- it's a complete crock, and will break the
>> instant someone looks at it sidewise.

> But it hasn't broken in years of use and maintenance, so that does not
> sound like an issue.

Uh, I was complaining about Bruce's idea of scribbling on the datetkn
table and expecting that to change what the datecache records. That's
not something we've been doing for years and years, and no I don't think
it's maintainable.

regards, tom lane

> Uh, I was complaining about Bruce's idea of scribbling on the datetkn
> table and expecting that to change what the datecache records. That's
> not something we've been doing for years and years, and no I don't think
> it's maintainable.

Thanks for the clarification on what you actually meant.

- Thomas