From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Gustafsson <daniel@yesql.se>
cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
        Robert Haas <robertmhaas@gmail.com>,
        Alvaro Herrera <alvherre@alvh.no-ip.org>,
        pgsql-hackers@lists.postgresql.org
Subject: Re: initdb's -c option behaves wrong way?
In-reply-to: <8103BBF3-107B-4FE8-B325-B9FC2BF7429D@yesql.se>
References: <2062980.1705523170@sss.pgh.pa.us>
 <2063810.1705523582@sss.pgh.pa.us>
 <2FC1625E-4757-4076-8965-7DBDA5ECC6D9@yesql.se>
 <20240118.134944.899707778910524729.horikyota.ntt@gmail.com>
 <8103BBF3-107B-4FE8-B325-B9FC2BF7429D@yesql.se>
Comments: In-reply-to Daniel Gustafsson <daniel@yesql.se>
	message dated "Fri, 19 Jan 2024 15:53:59 +0100"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <2629562.1705681924.0@sss.pgh.pa.us>
Date: Fri, 19 Jan 2024 11:33:57 -0500
Message-ID: <2629788.1705682037@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2629788.1705682037%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2629562.1705681924.1@sss.pgh.pa.us>

Daniel Gustafsson <daniel@yesql.se> writes:
> I'll give some more time for opinions, then I'll go ahead with one of the
> patches with a backpatch to v16.

OK, I take back my previous complaint that just using strncasecmp
would be enough --- I was misremembering how the code worked, and
you're right that it would use the spelling from the command line
rather than that from the file.

However, the v3 patch is flat broken.  You can't assume you have
found a match until you've verified that whitespace and '='
appear next --- otherwise, you'll be fooled by a guc_name that
is a prefix of one that appears in the file.  I think the simplest
change that does it correctly is as attached.

Now, since I was the one who wrote the existing code, I freely
concede that I may have too high an opinion of its readability.
Maybe some larger refactoring is appropriate.  But I didn't
find that what you'd done improved the readability.  I'd still
rather keep the newline-assembly code together as much as possible.
Maybe we should do the search part before we build any of newline?

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="v4-0001-Make-initdb-c-option-case-insensitive.patch";
	charset="us-ascii"
Content-ID: <2629562.1705681924.2@sss.pgh.pa.us>
Content-Description: v4-0001-Make-initdb-c-option-case-insensitive.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index ac409b0006..6a620c9d5f 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -484,6 +484,7 @@ replace_guc_value(char **lines, const char *guc_name, =
const char *guc_value,
 	for (i =3D 0; lines[i]; i++)
 	{
 		const char *where;
+		const char *namestart;
 =

 		/*
 		 * Look for a line assigning to guc_name.  Typically it will be
@@ -494,15 +495,19 @@ replace_guc_value(char **lines, const char *guc_name=
, const char *guc_value,
 		where =3D lines[i];
 		while (*where =3D=3D '#' || isspace((unsigned char) *where))
 			where++;
-		if (strncmp(where, guc_name, namelen) !=3D 0)
+		if (strncasecmp(where, guc_name, namelen) !=3D 0)
 			continue;
+		namestart =3D where;
 		where +=3D namelen;
 		while (isspace((unsigned char) *where))
 			where++;
 		if (*where !=3D '=3D')
 			continue;
 =

-		/* found it -- append the original comment if any */
+		/* found it -- let's use the canonical casing shown in the file */
+		memcpy(&newline->data[mark_as_comment ? 1 : 0], namestart, namelen);
+
+		/* now append the original comment if any */
 		where =3D strrchr(where, '#');
 		if (where)
 		{
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.=
pl
index 03376cc0f7..413a5eca67 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -199,4 +199,17 @@ command_fails(
 command_fails([ 'initdb', '--no-sync', '--set', 'foo=3Dbar', "$tempdir/da=
taX" ],
 	'fails for invalid --set option');
 =

+# Make sure multiple invocations of -c parameters are added case insensit=
ive
+command_ok(
+	[
+		'initdb', '-cwork_mem=3D128', '-cWork_Mem=3D256', '-cWORK_MEM=3D512',
+		"$tempdir/dataY"
+	],
+	'multiple -c options with different case');
+
+my $conf =3D slurp_file("$tempdir/dataY/postgresql.conf");
+ok($conf !~ qr/^WORK_MEM =3D /m, "WORK_MEM should not be configured");
+ok($conf !~ qr/^Work_Mem =3D /m, "Work_Mem should not be configured");
+ok($conf =3D~ qr/^work_mem =3D 512/m, "work_mem should be in config");
+
 done_testing();

------- =_aaaaaaaaaa0--