Make initdb throw error for bad locale values.

Historically we've printed a complaint for a bad locale setting, but then
fallen back to the environment default.  Per discussion, this is not such
a great idea, because rectifying an erroneous locale choice post-initdb
(perhaps long after data has been loaded) could be enormously expensive.
Better to complain and give the user a chance to double-check things.

The behavior was particularly bad if the bad setting came from environment
variables rather than a bogus command-line switch: in that case not only
was there a fallback to C/SQL_ASCII, but the printed complaint was quite
unhelpful.  It's hard to be entirely sure what variables setlocale looked
at, but we can at least give a hint where the problem might be.

Per a complaint from Tomas Vondra.

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 2a51916106b349f32e4af966ff0914ab2f32ba75..ec3b5e1876ceb4db88ac81e3d2c0bf4257697570 100644 (file)
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -252,7 +252,7 @@ static void trapsig(int signum);
 static void check_ok(void);
 static char *escape_quotes(const char *src);
 static int locale_date_order(const char *locale);
-static bool check_locale_name(int category, const char *locale,
+static void check_locale_name(int category, const char *locale,
                  char **canonname);
 static bool check_locale_encoding(const char *locale, int encoding);
 static void setlocales(void);
@@ -2529,7 +2529,7 @@ locale_date_order(const char *locale)
 }
 
 /*
- * Is the locale name valid for the locale category?
+ * Verify that locale name is valid for the locale category.
  *
  * If successful, and canonname isn't NULL, a malloc'd copy of the locale's
  * canonical name is stored there.  This is especially useful for figuring out
@@ -2540,7 +2540,7 @@ locale_date_order(const char *locale)
  *
  * this should match the backend's check_locale() function
  */
-static bool
+static void
 check_locale_name(int category, const char *locale, char **canonname)
 {
    char       *save;
@@ -2551,7 +2551,11 @@ check_locale_name(int category, const char *locale, char **canonname)
 
    save = setlocale(category, NULL);
    if (!save)
-       return false;           /* won't happen, we hope */
+   {
+       fprintf(stderr, _("%s: setlocale failed\n"),
+               progname);
+       exit(1);
+   }
 
    /* save may be pointing at a modifiable scratch variable, so copy it. */
    save = pg_strdup(save);
@@ -2565,16 +2569,34 @@ check_locale_name(int category, const char *locale, char **canonname)
 
    /* restore old value. */
    if (!setlocale(category, save))
+   {
        fprintf(stderr, _("%s: failed to restore old locale \"%s\"\n"),
                progname, save);
+       exit(1);
+   }
    free(save);
 
-   /* should we exit here? */
+   /* complain if locale wasn't valid */
    if (res == NULL)
-       fprintf(stderr, _("%s: invalid locale name \"%s\"\n"),
-               progname, locale);
-
-   return (res != NULL);
+   {
+       if (*locale)
+           fprintf(stderr, _("%s: invalid locale name \"%s\"\n"),
+                   progname, locale);
+       else
+       {
+           /*
+            * If no relevant switch was given on command line, locale is an
+            * empty string, which is not too helpful to report.  Presumably
+            * setlocale() found something it did not like in the environment.
+            * Ideally we'd report the bad environment variable, but since
+            * setlocale's behavior is implementation-specific, it's hard to
+            * be sure what it didn't like.  Print a safe generic message.
+            */
+           fprintf(stderr, _("%s: invalid locale settings; check LANG and LC_* environment variables\n"),
+                   progname);
+       }
+       exit(1);
+   }
 }
 
 /*
@@ -2642,41 +2664,27 @@ setlocales(void)
    }
 
    /*
-    * canonicalize locale names, and override any missing/invalid values from
-    * our current environment
+    * canonicalize locale names, and obtain any missing values from our
+    * current environment
     */
 
-   if (check_locale_name(LC_CTYPE, lc_ctype, &canonname))
-       lc_ctype = canonname;
-   else
-       lc_ctype = pg_strdup(setlocale(LC_CTYPE, NULL));
-   if (check_locale_name(LC_COLLATE, lc_collate, &canonname))
-       lc_collate = canonname;
-   else
-       lc_collate = pg_strdup(setlocale(LC_COLLATE, NULL));
-   if (check_locale_name(LC_NUMERIC, lc_numeric, &canonname))
-       lc_numeric = canonname;
-   else
-       lc_numeric = pg_strdup(setlocale(LC_NUMERIC, NULL));
-   if (check_locale_name(LC_TIME, lc_time, &canonname))
-       lc_time = canonname;
-   else
-       lc_time = pg_strdup(setlocale(LC_TIME, NULL));
-   if (check_locale_name(LC_MONETARY, lc_monetary, &canonname))
-       lc_monetary = canonname;
-   else
-       lc_monetary = pg_strdup(setlocale(LC_MONETARY, NULL));
+   check_locale_name(LC_CTYPE, lc_ctype, &canonname);
+   lc_ctype = canonname;
+   check_locale_name(LC_COLLATE, lc_collate, &canonname);
+   lc_collate = canonname;
+   check_locale_name(LC_NUMERIC, lc_numeric, &canonname);
+   lc_numeric = canonname;
+   check_locale_name(LC_TIME, lc_time, &canonname);
+   lc_time = canonname;
+   check_locale_name(LC_MONETARY, lc_monetary, &canonname);
+   lc_monetary = canonname;
 #if defined(LC_MESSAGES) && !defined(WIN32)
-   if (check_locale_name(LC_MESSAGES, lc_messages, &canonname))
-       lc_messages = canonname;
-   else
-       lc_messages = pg_strdup(setlocale(LC_MESSAGES, NULL));
+   check_locale_name(LC_MESSAGES, lc_messages, &canonname);
+   lc_messages = canonname;
 #else
    /* when LC_MESSAGES is not available, use the LC_CTYPE setting */
-   if (check_locale_name(LC_CTYPE, lc_messages, &canonname))
-       lc_messages = canonname;
-   else
-       lc_messages = pg_strdup(setlocale(LC_CTYPE, NULL));
+   check_locale_name(LC_CTYPE, lc_messages, &canonname);
+   lc_messages = canonname;
 #endif
 }