Re: Win32 timezone matching - Mailing list pgsql-hackers

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