Commit 3e51b278db leaves lc_* conf lines as commented-out when
their value is "C". This leads to the following behavior.
$ echo LANG
ja_JP.UTF8
$ initdb --no-locale hoge
$ grep lc_ hoge/postgresql.conf
#lc_messages = 'C' # locale for system error message
#lc_monetary = 'C' # locale for monetary formatting
#lc_numeric = 'C' # locale for number formatting
#lc_time = 'C' # locale for time formatting
In this scenario, the postmaster ends up emitting log massages in
Japanese, which contradicts the documentation.
https://www.postgresql.org/docs/devel/app-initdb.html
> --locale=locale
> Sets the default locale for the database cluster. If this option is
> not specified, the locale is inherited from the environment that
> initdb runs in. Locale support is described in Section 24.1.
>
..
> --lc-messages=locale
> Like --locale, but only sets the locale in the specified category.
Here's a somewhat amusing case:
$ echo LANG
ja_JP.UTF8
$ initdb --lc_messages=C
$ grep lc_ hoge/postgresql.conf
#lc_messages = 'C' # locale for system error message
lc_monetary = 'ja_JP.UTF8' # locale for monetary formatting
lc_numeric = 'ja_JP.UTF8' # locale for number formatting
lc_time = 'ja_JP.UTF8' # locale for time formatting
Hmm. it seems that initdb replaces the values of all categories
*except the specified one*. This behavior seems incorrect to
me. initdb should replace the value when explicitly specified in the
command line. If you use -c lc_messages=C, it does perform the
expected behavior to some extent, but I believe this is a separate
matter.
I have doubts about not replacing these lines for purely cosmetic
reasons. In this mail, I've attached three possible solutions for the
original issue: the first one enforces replacement only when specified
on the command line, the second one simply always performs
replacement, and the last one addresses the concern about the absence
of quotes around "C" by allowing explicit specification. (FWIW, I
prefer the last one.)
What do you think about these?
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 0c6f5ceb0a..56a8c5b60e 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -144,6 +144,10 @@ static char *lc_monetary = NULL;
static char *lc_numeric = NULL;
static char *lc_time = NULL;
static char *lc_messages = NULL;
+static bool lc_monetary_specified = false;
+static bool lc_numeric_specified = false;
+static bool lc_time_specified = false;
+static bool lc_messages_specified = false;
static char locale_provider = COLLPROVIDER_LIBC;
static char *icu_locale = NULL;
static char *icu_rules = NULL;
@@ -1230,19 +1234,19 @@ setup_config(void)
* Hack: don't replace the LC_XXX GUCs when their value is 'C', because
* replace_guc_value will decide not to quote that, which looks strange.
*/
- if (strcmp(lc_messages, "C") != 0)
+ if (strcmp(lc_messages, "C") != 0 || lc_messages_specified)
conflines = replace_guc_value(conflines, "lc_messages",
lc_messages, false);
- if (strcmp(lc_monetary, "C") != 0)
+ if (strcmp(lc_monetary, "C") != 0 || lc_monetary_specified)
conflines = replace_guc_value(conflines, "lc_monetary",
lc_monetary, false);
- if (strcmp(lc_numeric, "C") != 0)
+ if (strcmp(lc_numeric, "C") != 0 || lc_numeric_specified)
conflines = replace_guc_value(conflines, "lc_numeric",
lc_numeric, false);
- if (strcmp(lc_time, "C") != 0)
+ if (strcmp(lc_time, "C") != 0 || lc_time_specified)
conflines = replace_guc_value(conflines, "lc_time",
lc_time, false);
@@ -2374,6 +2378,15 @@ setlocales(void)
icu_locale = locale;
}
+ /*
+ * At this point, null means that the value is not specifed in the
command
+ * line.
+ */
+ if (lc_numeric != NULL) lc_numeric_specified = true;
+ if (lc_time != NULL) lc_time_specified = true;
+ if (lc_monetary != NULL) lc_monetary_specified = true;
+ if (lc_messages != NULL) lc_messages_specified = true;
+
/*
* canonicalize locale names, and obtain any missing values from our
* current environment
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 0c6f5ceb0a..646e8f29f6 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -1226,25 +1226,17 @@ setup_config(void)
conflines = replace_guc_value(conflines, "shared_buffers",
repltok,
false);
- /*
- * Hack: don't replace the LC_XXX GUCs when their value is 'C', because
- * replace_guc_value will decide not to quote that, which looks strange.
- */
- if (strcmp(lc_messages, "C") != 0)
- conflines = replace_guc_value(conflines, "lc_messages",
-
lc_messages, false);
+ conflines = replace_guc_value(conflines, "lc_messages",
+ lc_messages,
false);
- if (strcmp(lc_monetary, "C") != 0)
- conflines = replace_guc_value(conflines, "lc_monetary",
-
lc_monetary, false);
+ conflines = replace_guc_value(conflines, "lc_monetary",
+ lc_monetary,
false);
- if (strcmp(lc_numeric, "C") != 0)
- conflines = replace_guc_value(conflines, "lc_numeric",
-
lc_numeric, false);
+ conflines = replace_guc_value(conflines, "lc_numeric",
+ lc_numeric,
false);
- if (strcmp(lc_time, "C") != 0)
- conflines = replace_guc_value(conflines, "lc_time",
-
lc_time, false);
+ conflines = replace_guc_value(conflines, "lc_time",
+ lc_time,
false);
switch (locale_date_order(lc_time))
{
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 0c6f5ceb0a..a7e459a20a 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -258,7 +258,7 @@ static char **replace_token(char **lines,
const char *token,
const char *replacement);
static char **replace_guc_value(char **lines,
const char
*guc_name, const char *guc_value,
- bool
mark_as_comment);
+ bool
mark_as_comment, bool force_quote);
static bool guc_value_requires_quotes(const char *guc_value);
static char **readfile(const char *path);
static void writefile(char *path, char **lines);
@@ -457,6 +457,8 @@ replace_token(char **lines, const char *token, const char
*replacement)
* This is used for fixing up cases where the effective default might not
* match what is in postgresql.conf.sample.
*
+ * If force_quote is true, always quotes the value.
+ *
* We assume there's at most one matching assignment. If we find no match,
* append a new line with the desired assignment.
*
@@ -465,7 +467,7 @@ replace_token(char **lines, const char *token, const char
*replacement)
*/
static char **
replace_guc_value(char **lines, const char *guc_name, const char *guc_value,
- bool mark_as_comment)
+ bool mark_as_comment, bool force_quote)
{
int namelen = strlen(guc_name);
PQExpBuffer newline = createPQExpBuffer();
@@ -475,7 +477,7 @@ replace_guc_value(char **lines, const char *guc_name, const
char *guc_value,
if (mark_as_comment)
appendPQExpBufferChar(newline, '#');
appendPQExpBuffer(newline, "%s = ", guc_name);
- if (guc_value_requires_quotes(guc_value))
+ if (force_quote || guc_value_requires_quotes(guc_value))
appendPQExpBuffer(newline, "'%s'", escape_quotes(guc_value));
else
appendPQExpBufferStr(newline, guc_value);
@@ -1215,7 +1217,7 @@ setup_config(void)
snprintf(repltok, sizeof(repltok), "%d", n_connections);
conflines = replace_guc_value(conflines, "max_connections",
- repltok,
false);
+ repltok,
false, false);
if ((n_buffers * (BLCKSZ / 1024)) % 1024 == 0)
snprintf(repltok, sizeof(repltok), "%dMB",
@@ -1224,27 +1226,19 @@ setup_config(void)
snprintf(repltok, sizeof(repltok), "%dkB",
n_buffers * (BLCKSZ / 1024));
conflines = replace_guc_value(conflines, "shared_buffers",
- repltok,
false);
+ repltok,
false, false);
- /*
- * Hack: don't replace the LC_XXX GUCs when their value is 'C', because
- * replace_guc_value will decide not to quote that, which looks strange.
- */
- if (strcmp(lc_messages, "C") != 0)
- conflines = replace_guc_value(conflines, "lc_messages",
-
lc_messages, false);
+ conflines = replace_guc_value(conflines, "lc_messages",
+ lc_messages,
false, true);
- if (strcmp(lc_monetary, "C") != 0)
- conflines = replace_guc_value(conflines, "lc_monetary",
-
lc_monetary, false);
+ conflines = replace_guc_value(conflines, "lc_monetary",
+ lc_monetary,
false, true);
- if (strcmp(lc_numeric, "C") != 0)
- conflines = replace_guc_value(conflines, "lc_numeric",
-
lc_numeric, false);
+ conflines = replace_guc_value(conflines, "lc_numeric",
+ lc_numeric,
false, true);
- if (strcmp(lc_time, "C") != 0)
- conflines = replace_guc_value(conflines, "lc_time",
-
lc_time, false);
+ conflines = replace_guc_value(conflines, "lc_time",
+ lc_time,
false, true);
switch (locale_date_order(lc_time))
{
@@ -1260,70 +1254,72 @@ setup_config(void)
break;
}
conflines = replace_guc_value(conflines, "datestyle",
- repltok,
false);
+ repltok,
false, false);
snprintf(repltok, sizeof(repltok), "pg_catalog.%s",
default_text_search_config);
conflines = replace_guc_value(conflines, "default_text_search_config",
- repltok,
false);
+ repltok,
false, false);
if (default_timezone)
{
conflines = replace_guc_value(conflines, "timezone",
-
default_timezone, false);
+
default_timezone, false, false);
conflines = replace_guc_value(conflines, "log_timezone",
-
default_timezone, false);
+
default_timezone, false, false);
}
conflines = replace_guc_value(conflines, "dynamic_shared_memory_type",
-
dynamic_shared_memory_type, false);
+
dynamic_shared_memory_type, false, false);
/* Caution: these depend on wal_segment_size_mb, they're not constants
*/
conflines = replace_guc_value(conflines, "min_wal_size",
-
pretty_wal_size(DEFAULT_MIN_WAL_SEGS), false);
+
pretty_wal_size(DEFAULT_MIN_WAL_SEGS),
+ false, false);
conflines = replace_guc_value(conflines, "max_wal_size",
-
pretty_wal_size(DEFAULT_MAX_WAL_SEGS), false);
+
pretty_wal_size(DEFAULT_MAX_WAL_SEGS),
+ false, false);
/*
* Fix up various entries to match the true compile-time defaults.
Since
* these are indeed defaults, keep the postgresql.conf lines commented.
*/
conflines = replace_guc_value(conflines, "unix_socket_directories",
-
DEFAULT_PGSOCKET_DIR, true);
+
DEFAULT_PGSOCKET_DIR, true, false);
conflines = replace_guc_value(conflines, "port",
-
DEF_PGPORT_STR, true);
+
DEF_PGPORT_STR, true, false);
#if DEFAULT_BACKEND_FLUSH_AFTER > 0
snprintf(repltok, sizeof(repltok), "%dkB",
DEFAULT_BACKEND_FLUSH_AFTER * (BLCKSZ / 1024));
conflines = replace_guc_value(conflines, "backend_flush_after",
- repltok,
true);
+ repltok,
true, false);
#endif
#if DEFAULT_BGWRITER_FLUSH_AFTER > 0
snprintf(repltok, sizeof(repltok), "%dkB",
DEFAULT_BGWRITER_FLUSH_AFTER * (BLCKSZ / 1024));
conflines = replace_guc_value(conflines, "bgwriter_flush_after",
- repltok,
true);
+ repltok,
true, false);
#endif
#if DEFAULT_CHECKPOINT_FLUSH_AFTER > 0
snprintf(repltok, sizeof(repltok), "%dkB",
DEFAULT_CHECKPOINT_FLUSH_AFTER * (BLCKSZ / 1024));
conflines = replace_guc_value(conflines, "checkpoint_flush_after",
- repltok,
true);
+ repltok,
true, false);
#endif
#ifndef USE_PREFETCH
conflines = replace_guc_value(conflines, "effective_io_concurrency",
- "0", true);
+ "0", true,
false);
#endif
#ifdef WIN32
conflines = replace_guc_value(conflines, "update_process_title",
- "off", true);
+ "off", true,
false);
#endif
/*
@@ -1336,7 +1332,7 @@ setup_config(void)
strcmp(authmethodlocal, "scram-sha-256") != 0))
{
conflines = replace_guc_value(conflines, "password_encryption",
-
"md5", false);
+
"md5", false, false);
}
/*
@@ -1348,7 +1344,7 @@ setup_config(void)
if (pg_dir_create_mode == PG_DIR_MODE_GROUP)
{
conflines = replace_guc_value(conflines, "log_file_mode",
-
"0640", false);
+
"0640", false, false);
}
/*
@@ -1359,7 +1355,7 @@ setup_config(void)
gnames = gnames->next, gvalues = gvalues->next)
{
conflines = replace_guc_value(conflines, gnames->str,
-
gvalues->str, false);
+
gvalues->str, false, false);
}
/* ... and write out the finished postgresql.conf file */
