Обсуждение: Win32 timezone matching

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

Win32 timezone matching

От
Magnus Hagander
Дата:
When diagnosing a problem for a guy in the german channel (with Stefan
as mediator :P), we've found a case where the timezone information in
the registry seem to be empty for some timezones. The current timezone
matching code will abort scanning when it comes across one of these,
thus claiming it can't match the timezone, and reverting to GMT.

We've seen some reports prevously that looked like this, but never
really managed to get it diagnosed. Having checked the registry on
this machine, it appears to simply be that "Std" and "Dlt" are missing
from some entries.

The attach patch changes our scan to skip to the next timezone when
this happens, instead of aborting. The only downtime I can see from
this is that in case there are a *lot* of broken timezones (like "all
of them"), well log a lot of warnings. But it will only happen on
server startup, so I think it's ok.

Comments?


--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Вложения

Re: Win32 timezone matching

От
Tom Lane
Дата:
Magnus Hagander <magnus@hagander.net> writes:
> When diagnosing a problem for a guy in the german channel (with Stefan
> as mediator :P), we've found a case where the timezone information in
> the registry seem to be empty for some timezones. The current timezone
> matching code will abort scanning when it comes across one of these,
> thus claiming it can't match the timezone, and reverting to GMT.

> We've seen some reports prevously that looked like this, but never
> really managed to get it diagnosed. Having checked the registry on
> this machine, it appears to simply be that "Std" and "Dlt" are missing
> from some entries.

> The attach patch changes our scan to skip to the next timezone when
> this happens, instead of aborting. The only downtime I can see from
> this is that in case there are a *lot* of broken timezones (like "all
> of them"), well log a lot of warnings. But it will only happen on
> server startup, so I think it's ok.

I'm not clear on this.  Would this patch fix a real seen-in-the-field
condition, or is it speculative?  In particular, if the loop had kept
going in the complainant's machine, would it have found another entry
that worked better?
        regards, tom lane


Re: Win32 timezone matching

От
Magnus Hagander
Дата:
On Tue, Apr 6, 2010 at 23:44, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> When diagnosing a problem for a guy in the german channel (with Stefan
>> as mediator :P), we've found a case where the timezone information in
>> the registry seem to be empty for some timezones. The current timezone
>> matching code will abort scanning when it comes across one of these,
>> thus claiming it can't match the timezone, and reverting to GMT.
>
>> We've seen some reports prevously that looked like this, but never
>> really managed to get it diagnosed. Having checked the registry on
>> this machine, it appears to simply be that "Std" and "Dlt" are missing
>> from some entries.
>
>> The attach patch changes our scan to skip to the next timezone when
>> this happens, instead of aborting. The only downtime I can see from
>> this is that in case there are a *lot* of broken timezones (like "all
>> of them"), well log a lot of warnings. But it will only happen on
>> server startup, so I think it's ok.
>
> I'm not clear on this.  Would this patch fix a real seen-in-the-field
> condition, or is it speculative?  In particular, if the loop had kept
> going in the complainant's machine, would it have found another entry
> that worked better?

Real, seen-in-the-field. It would proceed and eventually find the guys
Berlin timezone (Central European, which comes after Central Brazilian
which is the one that was missing the entries).

It does, however, turn out that the issue was fixed when he re-applied
the latest timezone update hotfix from microsoft, which appears to
rewrite that entire subkey. Not sure how well we can trust that though
- it's not like we're working off a documented key...

-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/


Re: Win32 timezone matching

От
Tom Lane
Дата:
Magnus Hagander <magnus@hagander.net> writes:
> On Tue, Apr 6, 2010 at 23:44, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'm not clear on this. �Would this patch fix a real seen-in-the-field
>> condition, or is it speculative? �In particular, if the loop had kept
>> going in the complainant's machine, would it have found another entry
>> that worked better?

> Real, seen-in-the-field. It would proceed and eventually find the guys
> Berlin timezone (Central European, which comes after Central Brazilian
> which is the one that was missing the entries).

Ah, of course.  People who actually wanted the Central Brazilian zone
are screwed, but they're screwed anyway, and there's no need to also
screw people who want a zone that happens to come later in the list.
+1 for the patch then.

> It does, however, turn out that the issue was fixed when he re-applied
> the latest timezone update hotfix from microsoft, which appears to
> rewrite that entire subkey. Not sure how well we can trust that though
> - it's not like we're working off a documented key...

In any case, we might as well make things smoother for people working
with unpatched systems, if it's such a small change.
        regards, tom lane


Re: Win32 timezone matching

От
Tom Lane
Дата:
Oh, another thought here: what is the effect of the combination of this
with your other proposal to add more timezones to the list?  In
particular, what happens if we use the extended list in an unpatched
system that doesn't know about those new zone names?  I'm wondering
if we *have* to make this change for that to behave sanely.  We might
also need to consider whether we want any log chatter in such a case.
        regards, tom lane


Re: Win32 timezone matching

От
Magnus Hagander
Дата:
On Wed, Apr 7, 2010 at 00:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Oh, another thought here: what is the effect of the combination of this
> with your other proposal to add more timezones to the list?  In
> particular, what happens if we use the extended list in an unpatched
> system that doesn't know about those new zone names?  I'm wondering
> if we *have* to make this change for that to behave sanely.  We might
> also need to consider whether we want any log chatter in such a case.

No, as long as the key *exists* we've always moved on to the next one
if it didn't match. It's only when it doesn't exist at all that we
aborted.

And if the system doesn't know about those names, it'll never show up.
We're mapping *from* windows to *us*. So if the zones aren't in
windows at all, there is no way for windows to give us that name.


-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/


Re: Win32 timezone matching

От
Tom Lane
Дата:
Magnus Hagander <magnus@hagander.net> writes:
> On Wed, Apr 7, 2010 at 00:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Oh, another thought here: what is the effect of the combination of this
>> with your other proposal to add more timezones to the list?

> [ none ]

Ah, right, I hadn't looked closely at the logic before.  Still have
two comments though:

* There are other error conditions that cause "break"s in that loop.
Should we change the others?

* There is a comment at the head of win32_tzmap[] explaining where the
data came from; you need to update that.
        regards, tom lane


Re: Win32 timezone matching

От
Magnus Hagander
Дата:
On Wed, Apr 7, 2010 at 00:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> On Wed, Apr 7, 2010 at 00:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Oh, another thought here: what is the effect of the combination of this
>>> with your other proposal to add more timezones to the list?
>
>> [ none ]
>
> Ah, right, I hadn't looked closely at the logic before.  Still have
> two comments though:
>
> * There are other error conditions that cause "break"s in that loop.
> Should we change the others?

The only other error condition is if we find a key but can't open it.
That indicates something that's *seriously* wrong, so I'm inclined to
keep that one. The other two break statements are actually for when we
*find* a match, so they just indicate that we pick the first match
that we find.

> * There is a comment at the head of win32_tzmap[] explaining where the
> data came from; you need to update that.

Right.


-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/


Re: Win32 timezone matching

От
Stefan Kaltenbrunner
Дата:
Magnus Hagander wrote:
> On Wed, Apr 7, 2010 at 00:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Magnus Hagander <magnus@hagander.net> writes:
>>> On Wed, Apr 7, 2010 at 00:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> Oh, another thought here: what is the effect of the combination of this
>>>> with your other proposal to add more timezones to the list?
>>> [ none ]
>> Ah, right, I hadn't looked closely at the logic before.  Still have
>> two comments though:
>>
>> * There are other error conditions that cause "break"s in that loop.
>> Should we change the others?
> 
> The only other error condition is if we find a key but can't open it.
> That indicates something that's *seriously* wrong, so I'm inclined to
> keep that one. The other two break statements are actually for when we
> *find* a match, so they just indicate that we pick the first match
> that we find.

hmm all that code makes me wonder a bit about a more general issue - is 
the "fallback to GMT if we fail to actually make sense of the right 
imezone to use" actually a good idea?
In the case of the person I helped diagnosing the issue pg was bundled 
into some commercial app and the only reason the user immediately 
noticed that something was "wrong" was because the app used something 
like "select now()" to display the current time in the GUI.
I would consider the failure to make sense of the registry on windows or  failure to figure timezone information out a
moreserious issue than a 
 
mere "WARNING" because depending on how you look at the issue it might 
actually cause silent data corruption.


Stefan


Re: Win32 timezone matching

От
Tom Lane
Дата:
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
> hmm all that code makes me wonder a bit about a more general issue - is 
> the "fallback to GMT if we fail to actually make sense of the right 
> imezone to use" actually a good idea?

What alternative are you proposing?  Failing to start the server doesn't
seem like an attractive choice.

> I would consider the failure to make sense of the registry on windows or 
>   failure to figure timezone information out a more serious issue than a 
> mere "WARNING" because depending on how you look at the issue it might 
> actually cause silent data corruption.

Somehow, if you're running a database on windoze, I doubt your data
integrity standards are that high.  In any case I'd rather get a bleat
about "why is the server running in GMT" than "my database won't start".
The former will be a lot easier to narrow down.
        regards, tom lane


Re: Win32 timezone matching

От
Stefan Kaltenbrunner
Дата:
Tom Lane wrote:
> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
>> hmm all that code makes me wonder a bit about a more general issue - is 
>> the "fallback to GMT if we fail to actually make sense of the right 
>> imezone to use" actually a good idea?
> 
> What alternative are you proposing?  Failing to start the server doesn't
> seem like an attractive choice.

why not? we do error out in a lot of other cases as well... Personally I 
find a hard and clear "something is wrong please fix" much more 
convinient than defaulting to something that is more or less completely 
arbitrary but well...

> 
>> I would consider the failure to make sense of the registry on windows or 
>>   failure to figure timezone information out a more serious issue than a 
>> mere "WARNING" because depending on how you look at the issue it might 
>> actually cause silent data corruption.
> 
> Somehow, if you're running a database on windoze, I doubt your data
> integrity standards are that high.  In any case I'd rather get a bleat
> about "why is the server running in GMT" than "my database won't start".
> The former will be a lot easier to narrow down.

heh - except that we fail in that department - The only (not really 
useful hint) that pg logged was:

"WARNUNG:  could not query value for 'std' to identify Windows timezone: 2"

which says nothing about "I failed to figure something sane out and so I 
have to fallback to GMT" (which is what the !WIN32 code path seems to be 
actually doing but not the WIN32 code).
And even from the vendor perspective getting a support call on "uhm the 
database for your app is not starting what logs should I look at" seems 
better than "hmm we are now 2 weeks in production and just noticed that 
all the timestamps are off by a few hours how can we fix our data?".
PostgreSQL is bundled with a lot of apps on windows these days so the 
enduser might not even aware of it (and look into the eventlog only to 
find a rather oddly phrased WARNING) unless it fails hard...




Stefan


Re: Win32 timezone matching

От
Robert Haas
Дата:
On Wed, Apr 7, 2010 at 12:20 PM, Stefan Kaltenbrunner
<stefan@kaltenbrunner.cc> wrote:
> Tom Lane wrote:
>>
>> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
>>>
>>> hmm all that code makes me wonder a bit about a more general issue - is
>>> the "fallback to GMT if we fail to actually make sense of the right imezone
>>> to use" actually a good idea?
>>
>> What alternative are you proposing?  Failing to start the server doesn't
>> seem like an attractive choice.
>
> why not? we do error out in a lot of other cases as well... Personally I
> find a hard and clear "something is wrong please fix" much more convinient
> than defaulting to something that is more or less completely arbitrary but
> well...

While I can understand why someone might want that behavior in some
cases, in other cases it might be a severe overreaction.

...Robert


Re: Win32 timezone matching

От
Magnus Hagander
Дата:
On Wed, Apr 7, 2010 at 7:04 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Apr 7, 2010 at 12:20 PM, Stefan Kaltenbrunner
> <stefan@kaltenbrunner.cc> wrote:
>> Tom Lane wrote:
>>>
>>> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
>>>>
>>>> hmm all that code makes me wonder a bit about a more general issue - is
>>>> the "fallback to GMT if we fail to actually make sense of the right imezone
>>>> to use" actually a good idea?
>>>
>>> What alternative are you proposing?  Failing to start the server doesn't
>>> seem like an attractive choice.
>>
>> why not? we do error out in a lot of other cases as well... Personally I
>> find a hard and clear "something is wrong please fix" much more convinient
>> than defaulting to something that is more or less completely arbitrary but
>> well...
>
> While I can understand why someone might want that behavior in some
> cases, in other cases it might be a severe overreaction.

I think the dangerous scenario is if it worked, and then stopped
working. In that case, the database will change it's behavior and it
might go unnoticed. If it's wrong on first install, it'll likely get
noticed..

-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/


Re: Win32 timezone matching

От
Stefan Kaltenbrunner
Дата:
Magnus Hagander wrote:
> On Wed, Apr 7, 2010 at 7:04 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Wed, Apr 7, 2010 at 12:20 PM, Stefan Kaltenbrunner
>> <stefan@kaltenbrunner.cc> wrote:
>>> Tom Lane wrote:
>>>> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
>>>>> hmm all that code makes me wonder a bit about a more general issue - is
>>>>> the "fallback to GMT if we fail to actually make sense of the right imezone
>>>>> to use" actually a good idea?
>>>> What alternative are you proposing?  Failing to start the server doesn't
>>>> seem like an attractive choice.
>>> why not? we do error out in a lot of other cases as well... Personally I
>>> find a hard and clear "something is wrong please fix" much more convinient
>>> than defaulting to something that is more or less completely arbitrary but
>>> well...
>> While I can understand why someone might want that behavior in some
>> cases, in other cases it might be a severe overreaction.
> 
> I think the dangerous scenario is if it worked, and then stopped
> working. In that case, the database will change it's behavior and it
> might go unnoticed. If it's wrong on first install, it'll likely get
> noticed..

yeah that is one aspect - and in talking to the OP he would have 
prefered the database not starting up at all, logging an error and a 
hint on setting a fixed timezone in the conf.
Even if if keep the current fallback behaviour we should at least fix 
the windows codepath to do the same as the unix codepath does - as in 
actually logging that the fallback to GMT happened...



Stefan



Re: Win32 timezone matching

От
Tom Lane
Дата:
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
> yeah that is one aspect - and in talking to the OP he would have 
> prefered the database not starting up at all, logging an error and a 
> hint on setting a fixed timezone in the conf.

Well, you started from the statement that this was an embedded copy
of postgres ... so most users might not even know it was there,
much less to check its postmaster log for problems.  I'm still of
the opinion that refusing to start is an overreaction.

> Even if if keep the current fallback behaviour we should at least fix 
> the windows codepath to do the same as the unix codepath does - as in 
> actually logging that the fallback to GMT happened...

+1 for that anyway.  There already are WARNING messages for the various
Windows failure cases, but compared to the Unix code
       ereport(LOG,               (errmsg("could not determine system time zone, defaulting to \"%s\"", "GMT"),
errhint("Youcan specify the correct timezone in postgresql.conf.")));
 

they lack either the note about defaulting to GMT or the hint.  I guess
we should add both of those to the failure cases in the Windows version
of identify_system_timezone.  Should we also change the WARNING errlevel
to LOG?  I think the latter is more likely to actually get into the log.
        regards, tom lane


Re: Win32 timezone matching

От
Tom Lane
Дата:
I wrote:
>         ereport(LOG,
>                 (errmsg("could not determine system time zone, defaulting to \"%s\"", "GMT"),

BTW, does anyone remember the reason for making "GMT" nonlocalizable
in these messages?  It seems more straightforward to do
               (errmsg("could not determine system time zone, defaulting to \"GMT\""),

I suppose we had a reason for doing it the first way but I can't see
what.  "GMT" seems a fairly English-centric way of referring to UTC
anyhow; translators might wish to put in "UTC" instead, or some other
spelling.  Shouldn't we let them?
        regards, tom lane


Re: Win32 timezone matching

От
Magnus Hagander
Дата:
On Wed, Apr 7, 2010 at 00:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> On Wed, Apr 7, 2010 at 00:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Oh, another thought here: what is the effect of the combination of this
>>> with your other proposal to add more timezones to the list?
>

I've applied the patch to add the missing timezone names.

Before I did that I wrote a small perlscript that reads pgtz.c and
compares what's there to what's in the registry of the current
machine. Turns out I had missed one, which is Argentina Standard Time.

This script should probably live in CVS, and be run when Microsoft
releases new timezone data. Where should I put it - src/timezone or
somewhere in src/tools? (it does read pgtz.c in the current directory,
but it doesn't actually edit the file - just outputs on stdout a list
of changes to be made to the file manually)


-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/


Re: Win32 timezone matching

От
Magnus Hagander
Дата:
On Wed, Apr 7, 2010 at 21:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
>>         ereport(LOG,
>>                 (errmsg("could not determine system time zone, defaulting to \"%s\"", "GMT"),
>
> BTW, does anyone remember the reason for making "GMT" nonlocalizable
> in these messages?  It seems more straightforward to do

Nope, can't recall that.


>                (errmsg("could not determine system time zone, defaulting to \"GMT\""),
>
> I suppose we had a reason for doing it the first way but I can't see
> what.  "GMT" seems a fairly English-centric way of referring to UTC
> anyhow; translators might wish to put in "UTC" instead, or some other
> spelling.  Shouldn't we let them?


UTC and GMT aren't actually the same thing. In fact, it might be more
sensible to fall back to UTC than GMT. Both in the message *and* the
code, in that case. They only differ in fractions of seconds, but we
do deal in fractions of seconds... It also carries the nice property
that it's *supposed* to be abbreviated the same way regardless of
language (which is why it's UTC and not CUT).

And either way, it's an abbreviation, and we don't normally translate
those, do we?


-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/


Re: Win32 timezone matching

От
Magnus Hagander
Дата:
On Wed, Apr 7, 2010 at 21:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
>> Even if if keep the current fallback behaviour we should at least fix
>> the windows codepath to do the same as the unix codepath does - as in
>> actually logging that the fallback to GMT happened...
>
> +1 for that anyway.  There already are WARNING messages for the various
> Windows failure cases, but compared to the Unix code
>
>        ereport(LOG,
>                (errmsg("could not determine system time zone, defaulting to \"%s\"", "GMT"),
>        errhint("You can specify the correct timezone in postgresql.conf.")));
>
> they lack either the note about defaulting to GMT or the hint.  I guess
> we should add both of those to the failure cases in the Windows version
> of identify_system_timezone.  Should we also change the WARNING errlevel
> to LOG?  I think the latter is more likely to actually get into the log.

You are suggesting adding this after the "could not find match"
message, correct? Not replacing it? Because if we replace it, we loose
the information of what we failed to match. So basically like
attached?

Also, would LOG be *more* likely to be seen than a WARNING? Why would that be?


--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Вложения

Re: Win32 timezone matching

От
Tom Lane
Дата:
[ back to this... ]

Magnus Hagander <magnus@hagander.net> writes:
> On Wed, Apr 7, 2010 at 21:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I suppose we had a reason for doing it the first way but I can't see
>> what. �"GMT" seems a fairly English-centric way of referring to UTC
>> anyhow; translators might wish to put in "UTC" instead, or some other
>> spelling. �Shouldn't we let them?

> UTC and GMT aren't actually the same thing.

Tell it to the zic people --- they are identical except for the zone
abbreviation itself, according to the zic database.  There might be some
pedantic argument for preferring the name "UTC", but I'm hesitant to
change that behavior just to satisfy pedants.

> And either way, it's an abbreviation, and we don't normally translate
> those, do we?

I think that might in fact have been the argument for the current coding
style; but it still seems like it's overcomplicating matters in order to
prevent translators from making a decision they should be perfectly
competent to make for themselves.  Anyway, it's not real important.
Since nobody else seems to be excited about changing that part,
I'll leave it alone.
        regards, tom lane


Re: Win32 timezone matching

От
Tom Lane
Дата:
Magnus Hagander <magnus@hagander.net> writes:
> On Wed, Apr 7, 2010 at 21:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> ... lack either the note about defaulting to GMT or the hint.  I guess
>> we should add both of those to the failure cases in the Windows version
>> of identify_system_timezone.  Should we also change the WARNING errlevel
>> to LOG?  I think the latter is more likely to actually get into the log.

> You are suggesting adding this after the "could not find match"
> message, correct? Not replacing it? Because if we replace it, we loose
> the information of what we failed to match. So basically like
> attached?

No, I was thinking more like the attached.  This changes the Unix code
to separate the info about the fallback timezone into errdetail, and
then makes the Windows messages follow that style.

> Also, would LOG be *more* likely to be seen than a WARNING? Why would that be?

Because that's how log levels sort for the postmaster log.  This isn't
an interactive warning --- we will never be executing this code in a
regular backend, only in the postmaster.

            regards, tom lane

Index: pgtz.c
===================================================================
RCS file: /cvsroot/pgsql/src/timezone/pgtz.c,v
retrieving revision 1.71
diff -c -r1.71 pgtz.c
*** pgtz.c    9 Apr 2010 11:49:51 -0000    1.71
--- pgtz.c    15 Apr 2010 01:40:39 -0000
***************
*** 498,505 ****
      if (std_zone_name[0] == '\0')
      {
          ereport(LOG,
!                 (errmsg("could not determine system time zone, defaulting to \"%s\"", "GMT"),
!         errhint("You can specify the correct timezone in postgresql.conf.")));
          return NULL;            /* go to GMT */
      }

--- 498,507 ----
      if (std_zone_name[0] == '\0')
      {
          ereport(LOG,
!                 (errmsg("could not determine system time zone"),
!                  errdetail("The PostgreSQL time zone will be set to \"%s\".",
!                            "GMT"),
!                  errhint("You can specify the correct timezone in postgresql.conf.")));
          return NULL;            /* go to GMT */
      }

***************
*** 533,541 ****
               (-std_ofs > 0) ? "+" : "", -std_ofs / 3600);

      ereport(LOG,
!          (errmsg("could not recognize system timezone, defaulting to \"%s\"",
!                  resultbuf),
!        errhint("You can specify the correct timezone in postgresql.conf.")));
      return resultbuf;
  }

--- 535,544 ----
               (-std_ofs > 0) ? "+" : "", -std_ofs / 3600);

      ereport(LOG,
!             (errmsg("could not recognize system timezone"),
!              errdetail("The PostgreSQL time zone will be set to \"%s\".",
!                        resultbuf),
!              errhint("You can specify the correct timezone in postgresql.conf.")));
      return resultbuf;
  }

***************
*** 1076,1084 ****

      if (!tm)
      {
!         ereport(WARNING,
!                 (errmsg_internal("could not determine current date/time: localtime failed")));
!         return NULL;
      }

      memset(tzname, 0, sizeof(tzname));
--- 1079,1090 ----

      if (!tm)
      {
!         ereport(LOG,
!                 (errmsg("could not identify system time zone: localtime() failed"),
!                  errdetail("The PostgreSQL time zone will be set to \"%s\".",
!                            "GMT"),
!                  errhint("You can specify the correct timezone in postgresql.conf.")));
!         return NULL;            /* go to GMT */
      }

      memset(tzname, 0, sizeof(tzname));
***************
*** 1089,1095 ****
          if (strcmp(tzname, win32_tzmap[i].stdname) == 0 ||
              strcmp(tzname, win32_tzmap[i].dstname) == 0)
          {
!             elog(DEBUG4, "TZ \"%s\" matches Windows timezone \"%s\"",
                   win32_tzmap[i].pgtzname, tzname);
              return win32_tzmap[i].pgtzname;
          }
--- 1095,1101 ----
          if (strcmp(tzname, win32_tzmap[i].stdname) == 0 ||
              strcmp(tzname, win32_tzmap[i].dstname) == 0)
          {
!             elog(DEBUG4, "TZ \"%s\" matches system time zone \"%s\"",
                   win32_tzmap[i].pgtzname, tzname);
              return win32_tzmap[i].pgtzname;
          }
***************
*** 1107,1115 ****
                       KEY_READ,
                       &rootKey) != ERROR_SUCCESS)
      {
!         ereport(WARNING,
!                 (errmsg_internal("could not open registry key to identify Windows timezone: %i", (int)
GetLastError())));
!         return NULL;
      }

      for (idx = 0;; idx++)
--- 1113,1125 ----
                       KEY_READ,
                       &rootKey) != ERROR_SUCCESS)
      {
!         ereport(LOG,
!                 (errmsg("could not open registry key to identify system time zone: %i",
!                         (int) GetLastError()),
!                  errdetail("The PostgreSQL time zone will be set to \"%s\".",
!                            "GMT"),
!                  errhint("You can specify the correct timezone in postgresql.conf.")));
!         return NULL;            /* go to GMT */
      }

      for (idx = 0;; idx++)
***************
*** 1134,1148 ****
          {
              if (r == ERROR_NO_MORE_ITEMS)
                  break;
!             ereport(WARNING,
!                     (errmsg_internal("could not enumerate registry subkeys to identify Windows timezone: %i", (int)
r)));
              break;
          }

          if ((r = RegOpenKeyEx(rootKey, keyname, 0, KEY_READ, &key)) != ERROR_SUCCESS)
          {
!             ereport(WARNING,
!                     (errmsg_internal("could not open registry subkey to identify Windows timezone: %i", (int) r)));
              break;
          }

--- 1144,1158 ----
          {
              if (r == ERROR_NO_MORE_ITEMS)
                  break;
!             ereport(LOG,
!                     (errmsg_internal("could not enumerate registry subkeys to identify system time zone: %i", (int)
r)));
              break;
          }

          if ((r = RegOpenKeyEx(rootKey, keyname, 0, KEY_READ, &key)) != ERROR_SUCCESS)
          {
!             ereport(LOG,
!                     (errmsg_internal("could not open registry subkey to identify system time zone: %i", (int) r)));
              break;
          }

***************
*** 1150,1157 ****
          namesize = sizeof(zonename);
          if ((r = RegQueryValueEx(key, "Std", NULL, NULL, zonename, &namesize)) != ERROR_SUCCESS)
          {
!             ereport(WARNING,
!                     (errmsg_internal("could not query value for 'std' to identify Windows timezone \"%s\": %i",
                                       keyname, (int) r)));
              RegCloseKey(key);
              continue; /* Proceed to look at the next timezone */
--- 1160,1167 ----
          namesize = sizeof(zonename);
          if ((r = RegQueryValueEx(key, "Std", NULL, NULL, zonename, &namesize)) != ERROR_SUCCESS)
          {
!             ereport(LOG,
!                     (errmsg_internal("could not query value for key \"std\" to identify system time zone \"%s\": %i",
                                       keyname, (int) r)));
              RegCloseKey(key);
              continue; /* Proceed to look at the next timezone */
***************
*** 1167,1174 ****
          namesize = sizeof(zonename);
          if ((r = RegQueryValueEx(key, "Dlt", NULL, NULL, zonename, &namesize)) != ERROR_SUCCESS)
          {
!             ereport(WARNING,
!                     (errmsg_internal("could not query value for 'dlt' to identify Windows timezone \"%s\": %i",
                                       keyname, (int) r)));
              RegCloseKey(key);
              continue; /* Proceed to look at the next timezone */
--- 1177,1184 ----
          namesize = sizeof(zonename);
          if ((r = RegQueryValueEx(key, "Dlt", NULL, NULL, zonename, &namesize)) != ERROR_SUCCESS)
          {
!             ereport(LOG,
!                     (errmsg_internal("could not query value for key \"dlt\" to identify system time zone \"%s\": %i",
                                       keyname, (int) r)));
              RegCloseKey(key);
              continue; /* Proceed to look at the next timezone */
***************
*** 1194,1210 ****
              if (strcmp(localtzname, win32_tzmap[i].stdname) == 0 ||
                  strcmp(localtzname, win32_tzmap[i].dstname) == 0)
              {
!                 elog(DEBUG4, "TZ \"%s\" matches localized Windows timezone \"%s\" (\"%s\")",
                       win32_tzmap[i].pgtzname, tzname, localtzname);
                  return win32_tzmap[i].pgtzname;
              }
          }
      }

!     ereport(WARNING,
!             (errmsg("could not find a match for Windows timezone \"%s\"",
!                     tzname)));
!     return NULL;
  }
  #endif   /* WIN32 */

--- 1204,1223 ----
              if (strcmp(localtzname, win32_tzmap[i].stdname) == 0 ||
                  strcmp(localtzname, win32_tzmap[i].dstname) == 0)
              {
!                 elog(DEBUG4, "TZ \"%s\" matches localized system time zone \"%s\" (\"%s\")",
                       win32_tzmap[i].pgtzname, tzname, localtzname);
                  return win32_tzmap[i].pgtzname;
              }
          }
      }

!     ereport(LOG,
!             (errmsg("could not find a match for system time zone \"%s\"",
!                     tzname),
!              errdetail("The PostgreSQL time zone will be set to \"%s\".",
!                        "GMT"),
!              errhint("You can specify the correct timezone in postgresql.conf.")));
!     return NULL;            /* go to GMT */
  }
  #endif   /* WIN32 */


Re: Win32 timezone matching

От
Magnus Hagander
Дата:
On Thu, Apr 15, 2010 at 2:54 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> [ back to this... ]
>
> Magnus Hagander <magnus@hagander.net> writes:
>> On Wed, Apr 7, 2010 at 21:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I suppose we had a reason for doing it the first way but I can't see
>>> what.  "GMT" seems a fairly English-centric way of referring to UTC
>>> anyhow; translators might wish to put in "UTC" instead, or some other
>>> spelling.  Shouldn't we let them?
>
>> UTC and GMT aren't actually the same thing.
>
> Tell it to the zic people --- they are identical except for the zone
> abbreviation itself, according to the zic database.  There might be some
> pedantic argument for preferring the name "UTC", but I'm hesitant to
> change that behavior just to satisfy pedants.

Agreed, I don't think it's worth changing. However, that also goes to
the translation of it - let's keep *one* term, that'll make it a lot
less confusing.


-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/


Re: Win32 timezone matching

От
Magnus Hagander
Дата:
On Thu, Apr 15, 2010 at 3:48 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Magnus Hagander <magnus@hagander.net> writes:
>> On Wed, Apr 7, 2010 at 21:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> ... lack either the note about defaulting to GMT or the hint.  I guess
>>> we should add both of those to the failure cases in the Windows version
>>> of identify_system_timezone.  Should we also change the WARNING errlevel
>>> to LOG?  I think the latter is more likely to actually get into the log.
>
>> You are suggesting adding this after the "could not find match"
>> message, correct? Not replacing it? Because if we replace it, we loose
>> the information of what we failed to match. So basically like
>> attached?
>
> No, I was thinking more like the attached.  This changes the Unix code
> to separate the info about the fallback timezone into errdetail, and
> then makes the Windows messages follow that style.

Yeah, that looks good.


>> Also, would LOG be *more* likely to be seen than a WARNING? Why would that be?
>
> Because that's how log levels sort for the postmaster log.  This isn't
> an interactive warning --- we will never be executing this code in a
> regular backend, only in the postmaster.

Well, when the dba looks through the log, he'll be looking a lot
harder at something that says WARNING.

And if somebody is filtering his log so hard that it doesn't even
contain WARNING's, frankly, he's ignorant ;)

But that's just me, and I've never really agreed with that soring in
the first place, so maybe I should just be ignored...


-- Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/