Обсуждение: Win32 timezone matching
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/
Вложения
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
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/
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
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
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/
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
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/
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
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
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
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
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/
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
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
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
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/
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/
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/
Вложения
[ 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
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 */
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/
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/