Date: Thu, 28 Sep 2023 16:49:04 +0900 (JST)
Message-Id: <20230928.164904.2153358973162534034.horikyota.ntt@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Subject: initdb's -c option behaves wrong way?
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
User-Agent: Mew version 6.8 on Emacs 27.2
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Thu_Sep_28_16_49_04_2023_929)--"
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20230928.164904.2153358973162534034.horikyota.ntt%40gmail.com>
Precedence: bulk

----Next_Part(Thu_Sep_28_16_49_04_2023_929)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hello.

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
...
$ grep -i 'work_mem ' $PGDATA/postgresql.conf
work_mem = 100                          # min 64kB
#maintenance_work_mem = 64MB            # min 1MB
#autovacuum_work_mem = -1               # min 1MB, or -1 to use maintenance_work_mem
#logical_decoding_work_mem = 64MB       # min 64kB
WORK_MEM = 1000
Work_mem = 2000


The original intention was apparently to overwrite the existing
line. Furthermore, I surmise that preserving the original letter
casing is preferable.

Attached is a patch to address this issue.  To retrieve the variable
name from the existing line, the code is slightly restructured.
Alternatively, should we just down-case the provided variable names?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

----Next_Part(Thu_Sep_28_16_49_04_2023_929)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="initdb-c_ignore_letter_case.patch"

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 0c6f5ceb0a..04419840f4 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -470,20 +470,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
@@ -493,15 +487,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);
+
+		/*
+		 * completed body of line, now continue with potential indentation and
+		 * comment
+		 */
 		where = strrchr(where, '#');
 		if (where)
 		{
@@ -548,16 +559,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;

----Next_Part(Thu_Sep_28_16_49_04_2023_929)----