Re: [COMMITTERS] pgsql-server/src backend/commands/variable.c b ...

CVSROOT: /cvsroot
Module name: pgsql-server
Changes by: tgl(at)developer(dot)postgresql(dot)org 03/05/17 21:06:26

Modified files:
src/backend/commands: variable.c
src/backend/utils/adt: datetime.c
src/include/utils: datetime.h

Log message:
Add code to test for unknown timezone names (following some ideas from
Ross Reedstrom, a couple months back) and to detect timezones that are
using leap-second timekeeping. The unknown-zone-name test is pretty
heuristic and ugly, but it seems better than the old behavior of just
switching to GMT given a bad name. Also make DecodePosixTimezone() a
tad more robust.

Tom, I am seeing compile failures because of this change. I don't have
a global timezone here but have the struct tm. I see in other uses of
timezone that we test for 'struct tm' and than if it doesn't exist, use
timezone.

Can you make that change?

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

Tom Lane wrote:
> CVSROOT: /cvsroot
> Module name: pgsql-server
> Changes by: tgl(at)developer(dot)postgresql(dot)org 03/05/17 21:06:26
>
> Modified files:
> src/backend/commands: variable.c
> src/backend/utils/adt: datetime.c
> src/include/utils: datetime.h
>
> Log message:
> Add code to test for unknown timezone names (following some ideas from
> Ross Reedstrom, a couple months back) and to detect timezones that are
> using leap-second timekeeping. The unknown-zone-name test is pretty
> heuristic and ugly, but it seems better than the old behavior of just
> switching to GMT given a bad name. Also make DecodePosixTimezone() a
> tad more robust.
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
>

--
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 seeing a compile failure in variable.c because it references
timezone, but I only have struct tm on BSD/OS.

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

Tom Lane wrote:
> CVSROOT: /cvsroot
> Module name: pgsql-server
> Changes by: tgl(at)developer(dot)postgresql(dot)org 03/05/17 21:06:26
>
> Modified files:
> src/backend/commands: variable.c
> src/backend/utils/adt: datetime.c
> src/include/utils: datetime.h
>
> Log message:
> Add code to test for unknown timezone names (following some ideas from
> Ross Reedstrom, a couple months back) and to detect timezones that are
> using leap-second timekeeping. The unknown-zone-name test is pretty
> heuristic and ugly, but it seems better than the old behavior of just
> switching to GMT given a bad name. Also make DecodePosixTimezone() a
> tad more robust.
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
>

--
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:
> I am seeing a compile failure in variable.c because it references
> timezone, but I only have struct tm on BSD/OS.

Hm. Shouldn't TIMEZONE_GLOBAL expand to something that exists
everywhere? What exact error are you getting?

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > I am seeing a compile failure in variable.c because it references
> > timezone, but I only have struct tm on BSD/OS.
>
> Hm. Shouldn't TIMEZONE_GLOBAL expand to something that exists
> everywhere? What exact error are you getting?

I am getting:

gcc -O2 -pipe -m486 -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 variable.o variable.c
variable.c: In function `tzset_succeeded':
variable.c:328: `tzname' undeclared (first use in this function)
variable.c:328: (Each undeclared identifier is reported only once
variable.c:328: for each function it appears in.)
gmake: *** [variable.o] Error 1

It is tzname. Sorry, I said timezone, but I meant timezone name.

--
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:
> I am getting:

> gcc -O2 -pipe -m486 -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 variable.o variable.c
> variable.c: In function `tzset_succeeded':
> variable.c:328: `tzname' undeclared (first use in this function)

Odd. tzname is supposed to exist according to the Unix specs I've
looked at, eg
http://www.opengroup.org/onlinepubs/007908799/xsh/tzname.html
It does exist on Marc's FreeBSD setup:
> gcc -E /usr/include/time.h | grep tzname
extern char *tzname[];

Maybe on your machine we need to include <sys/time.h> or some such?

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > I am getting:
>
> > gcc -O2 -pipe -m486 -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 variable.o variable.c
> > variable.c: In function `tzset_succeeded':
> > variable.c:328: `tzname' undeclared (first use in this function)
>
> Odd. tzname is supposed to exist according to the Unix specs I've
> looked at, eg
> http://www.opengroup.org/onlinepubs/007908799/xsh/tzname.html
> It does exist on Marc's FreeBSD setup:
> > gcc -E /usr/include/time.h | grep tzname
> extern char *tzname[];
>
> Maybe on your machine we need to include <sys/time.h> or some such?

I just checked using grep on all my /usr/include files, and can't find
tzname anywhere, but it is in libc. I added a line to port/bsdi.h to
fix it, but I am not sure that is the correct solution.

I thought this might be new because of my upgrade to BSD/OS 4.3, but I
just checked my 4.2 machine and it also doesn't have tzname. Seems this
might be the first time we are referencing tzname if struct tm already
exists. Can we switch to a struct tm/tzname solution?

--
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:
> I thought this might be new because of my upgrade to BSD/OS 4.3, but I
> just checked my 4.2 machine and it also doesn't have tzname. Seems this
> might be the first time we are referencing tzname if struct tm already
> exists. Can we switch to a struct tm/tzname solution?

Don't see how --- tzset doesn't return a struct tm. We may have to just
not make the test on machines without tzname[]. Grotty, but they're no
worse off than before...

regards, tom lane

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Tom Lane wrote:
>> Maybe on your machine we need to include <sys/time.h> or some such?

> I just checked using grep on all my /usr/include files, and can't find
> tzname anywhere, but it is in libc. I added a line to port/bsdi.h to
> fix it, but I am not sure that is the correct solution.

I looked at the autoconf macro that tests for existence of tzname[],
and saw that it explicitly does

#ifndef tzname /* For SGI. */
extern char *tzname[];
#endif

after including <time.h>. So it seems that they are expecting some
systems not to provide a declaration for tzname even though they have it.

I have checked in a fix that does #ifdef HAVE_TZNAME, and includes
a declaration as above. (Without the declaration, we would fail anyway
on machines that are like this.) So you shouldn't need the port/bsdi.h
hack anymore.

regards, tom lane

OK, thanks.

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

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > Tom Lane wrote:
> >> Maybe on your machine we need to include <sys/time.h> or some such?
>
> > I just checked using grep on all my /usr/include files, and can't find
> > tzname anywhere, but it is in libc. I added a line to port/bsdi.h to
> > fix it, but I am not sure that is the correct solution.
>
> I looked at the autoconf macro that tests for existence of tzname[],
> and saw that it explicitly does
>
> #ifndef tzname /* For SGI. */
> extern char *tzname[];
> #endif
>
> after including <time.h>. So it seems that they are expecting some
> systems not to provide a declaration for tzname even though they have it.
>
> I have checked in a fix that does #ifdef HAVE_TZNAME, and includes
> a declaration as above. (Without the declaration, we would fail anyway
> on machines that are like this.) So you shouldn't need the port/bsdi.h
> hack anymore.
>
> regards, tom lane
>

--
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