public inbox for [email protected]
help / color / mirror / Atom feedFrom: Daniel Gustafsson <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Subject: Re: initdb's -c option behaves wrong way?
Date: Tue, 16 Jan 2024 12:17:23 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
> On 28 Sep 2023, at 09:49, Kyotaro Horiguchi <[email protected]> wrote:
> I noticed that -c option of initdb behaves in an unexpected
> manner. Identical variable names with variations in letter casing are
> treated as distinct variables.
>
> $ initdb -cwork_mem=100 -cWORK_MEM=1000 -cWork_mem=2000
> The original intention was apparently to overwrite the existing
> line. Furthermore, I surmise that preserving the original letter
> casing is preferable.
Circling back to an old thread, I agree that this seems odd and the original
thread [0] makes no mention of it being intentional.
The patch seems fine to me, the attached version is rebased, pgindented and has
a test case added.
--
Daniel Gustafsson
[0] https://www.postgresql.org/message-id/flat/2844176.1674681919%40sss.pgh.pa.us
Attachments:
[application/octet-stream] v2-0001-Make-initdb-c-option-case-insensitive.patch (4.3K, ../[email protected]/2-v2-0001-Make-initdb-c-option-case-insensitive.patch)
download | inline diff:
From cbdfe0eefbd2707401f542e3e5a4e08c7202a77c Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <[email protected]>
Date: Tue, 16 Jan 2024 12:02:21 +0100
Subject: [PATCH v2] Make initdb -c option case insensitive
When a -c option specifies a GUC name to replace using different
case than the existing parameter it was added as a new parameter
rather than replacing the existing. This makes -c replacements
case insensitive such that -cWORK_MEM will update the value of
work_mem in the config.
Reported-by: Kyotaro Horiguchi <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/bin/initdb/initdb.c | 46 ++++++++++++++++++++++------------
src/bin/initdb/t/001_initdb.pl | 12 +++++++++
2 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index ac409b0006..aa858ece8a 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -471,20 +471,14 @@ replace_guc_value(char **lines, const char *guc_name, const char *guc_value,
int namelen = strlen(guc_name);
PQExpBuffer newline = createPQExpBuffer();
int i;
+ const char *where;
+ const char *pname;
- /* prepare the replacement line, except for possible comment and newline */
if (mark_as_comment)
appendPQExpBufferChar(newline, '#');
- appendPQExpBuffer(newline, "%s = ", guc_name);
- if (guc_value_requires_quotes(guc_value))
- appendPQExpBuffer(newline, "'%s'", escape_quotes(guc_value));
- else
- appendPQExpBufferStr(newline, guc_value);
for (i = 0; lines[i]; i++)
{
- const char *where;
-
/*
* Look for a line assigning to guc_name. Typically it will be
* preceded by '#', but that might not be the case if a -c switch
@@ -494,15 +488,32 @@ replace_guc_value(char **lines, const char *guc_name, const char *guc_value,
where = lines[i];
while (*where == '#' || isspace((unsigned char) *where))
where++;
- if (strncmp(where, guc_name, namelen) != 0)
+ if (strncasecmp(where, guc_name, namelen) != 0)
continue;
+
+ pname = where;
where += namelen;
while (isspace((unsigned char) *where))
where++;
- if (*where != '=')
- continue;
- /* found it -- append the original comment if any */
+ /* assume there's only one match */
+ if (*where == '=')
+ break;
+ }
+
+ if (lines[i])
+ {
+ /* Found it, rewrite the line preserving the original comment if any */
+ appendPQExpBuffer(newline, "%.*s = ", namelen, pname);
+ if (guc_value_requires_quotes(guc_value))
+ appendPQExpBuffer(newline, "'%s'", escape_quotes(guc_value));
+ else
+ appendPQExpBufferStr(newline, guc_value);
+
+ /*
+ * Body of line has been completed, now continue with potential
+ * indentation and comment.
+ */
where = strrchr(where, '#');
if (where)
{
@@ -549,16 +560,19 @@ replace_guc_value(char **lines, const char *guc_name, const char *guc_value,
free(lines[i]);
lines[i] = newline->data;
-
- break; /* assume there's only one match */
}
-
- if (lines[i] == NULL)
+ else
{
/*
* No match, so append a new entry. (We rely on the bootstrap server
* to complain if it's not a valid GUC name.)
*/
+ appendPQExpBuffer(newline, "%s = ", guc_name);
+ if (guc_value_requires_quotes(guc_value))
+ appendPQExpBuffer(newline, "'%s'", escape_quotes(guc_value));
+ else
+ appendPQExpBufferStr(newline, guc_value);
+
appendPQExpBufferChar(newline, '\n');
lines = pg_realloc_array(lines, char *, i + 2);
lines[i++] = newline->data;
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 03376cc0f7..1c048bba8e 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -199,4 +199,16 @@ command_fails(
command_fails([ 'initdb', '--no-sync', '--set', 'foo=bar', "$tempdir/dataX" ],
'fails for invalid --set option');
+# Make sure multiple invocations of -c parameters are added case insensitive
+command_ok(
+ [
+ 'initdb', '-cwork_mem=128', '-cWork_Mem=256', '-cWORK_MEM=512',
+ "$tempdir/dataX"
+ ],
+ 'multiple -c options with different case');
+
+my $conf = slurp_file("$tempdir/dataX/postgresql.conf");
+ok($conf =~ qr/^work_mem = 512/m, "work_mem configured");
+ok($conf !~ qr/^WORK_MEM = 512/m, "WORK_MEM should not be in config");
+
done_testing();
--
2.32.1 (Apple Git-133)
view thread (14+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: initdb's -c option behaves wrong way?
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox