From: Daniel Gustafsson <daniel@yesql.se>
Message-Id: <2FC1625E-4757-4076-8965-7DBDA5ECC6D9@yesql.se>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_9445B444-FE53-4C09-A2A4-B84EFDD728E1"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.4\))
Subject: Re: initdb's -c option behaves wrong way?
Date: Wed, 17 Jan 2024 23:47:41 +0100
In-Reply-To: <2063810.1705523582@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>,
 Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 pgsql-hackers@lists.postgresql.org
To: Tom Lane <tgl@sss.pgh.pa.us>
References: <202401171615.uzdmkozzvo6x@alvherre.pgsql>
 <2040188.1705511127@sss.pgh.pa.us>
 <5DFF3111-8ECA-4EF5-B614-9A4C9FAB29BC@yesql.se>
 <CA+TgmoZYa5hNgdadYwNfpReykW7RU5egpzrFaAMwFA9qz0g=eg@mail.gmail.com>
 <2062980.1705523170@sss.pgh.pa.us> <2063810.1705523582@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2FC1625E-4757-4076-8965-7DBDA5ECC6D9%40yesql.se>
Precedence: bulk


--Apple-Mail=_9445B444-FE53-4C09-A2A4-B84EFDD728E1
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

> On 17 Jan 2024, at 21:33, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>=20
> I wrote:
>> However ... I don't like the patch much.  It seems to have left
>> the code in a rather random state.  Why, for example, didn't you
>> keep all the code that constructs the "newline" value together?
>=20
> After thinking about it a bit more, I don't see why you didn't just
> s/strncmp/strncasecmp/ and call it good.  The messiness seems to be
> a result of your choice to replace the GUC's case as shown in the
> file with the case used on the command line, which is not better
> IMO.  We don't change our mind about the canonical spelling of a
> GUC because somebody varied the case in a SET command.

The original patch was preserving the case of the file throwing away the =
case
from the commandline.  The attached is a slimmed down version which only =
moves
the assembly of newline to the different cases (replace vs.  new) =
keeping the
rest of the code intact.  Keeping it in one place gets sort of messy too =
since
it needs to use different values for a replacement and a new variable.  =
Not
sure if there is a cleaner approach?

--
Daniel Gustafsson


--Apple-Mail=_9445B444-FE53-4C09-A2A4-B84EFDD728E1
Content-Disposition: attachment;
	filename=v3-0001-Make-initdb-c-option-case-insensitive.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v3-0001-Make-initdb-c-option-case-insensitive.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20c71e123c19c7b486182f02375a623d0d8698d680=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20Daniel=20Gustafsson=20=
<dgustafsson@postgresql.org>=0ADate:=20Wed,=2017=20Jan=202024=2022:47:03=20=
+0100=0ASubject:=20[PATCH=20v3]=20Make=20initdb=20-c=20option=20case=20=
insensitive=0A=0AWhen=20a=20-c=20option=20specifies=20a=20GUC=20name=20=
to=20replace=20using=20different=0Acase=20than=20the=20existing=20=
parameter=20it=20was=20added=20as=20a=20new=20parameter=0Arather=20than=20=
replacing=20the=20existing.=20=20This=20makes=20-c=20replacements=0Acase=20=
insensitive=20such=20that=20-cWORK_MEM=20will=20replace=20the=20value=20=
of=0Awork_mem=20in=20the=20config.=0A=0AReported-by:=20Kyotaro=20=
Horiguchi=20<horikyota.ntt@gmail.com>=0ADiscussion:=20=
https://postgr.es/m/20230928.164904.2153358973162534034.horikyota.ntt@gmai=
l.com=0A---=0A=20src/bin/initdb/initdb.c=20=20=20=20=20=20=20=20|=2019=20=
++++++++++++-------=0A=20src/bin/initdb/t/001_initdb.pl=20|=2013=20=
+++++++++++++=0A=202=20files=20changed,=2025=20insertions(+),=207=20=
deletions(-)=0A=0Adiff=20--git=20a/src/bin/initdb/initdb.c=20=
b/src/bin/initdb/initdb.c=0Aindex=20ac409b0006..edfab4ed22=20100644=0A=
---=20a/src/bin/initdb/initdb.c=0A+++=20b/src/bin/initdb/initdb.c=0A@@=20=
-475,11=20+475,6=20@@=20replace_guc_value(char=20**lines,=20const=20char=20=
*guc_name,=20const=20char=20*guc_value,=0A=20=09/*=20prepare=20the=20=
replacement=20line,=20except=20for=20possible=20comment=20and=20newline=20=
*/=0A=20=09if=20(mark_as_comment)=0A=20=09=09=
appendPQExpBufferChar(newline,=20'#');=0A-=09appendPQExpBuffer(newline,=20=
"%s=20=3D=20",=20guc_name);=0A-=09if=20=
(guc_value_requires_quotes(guc_value))=0A-=09=09=
appendPQExpBuffer(newline,=20"'%s'",=20escape_quotes(guc_value));=0A-=09=
else=0A-=09=09appendPQExpBufferStr(newline,=20guc_value);=0A=20=0A=20=09=
for=20(i=20=3D=200;=20lines[i];=20i++)=0A=20=09{=0A@@=20-494,8=20+489,15=20=
@@=20replace_guc_value(char=20**lines,=20const=20char=20*guc_name,=20=
const=20char=20*guc_value,=0A=20=09=09where=20=3D=20lines[i];=0A=20=09=09=
while=20(*where=20=3D=3D=20'#'=20||=20isspace((unsigned=20char)=20=
*where))=0A=20=09=09=09where++;=0A-=09=09if=20(strncmp(where,=20=
guc_name,=20namelen)=20!=3D=200)=0A+=09=09if=20(strncasecmp(where,=20=
guc_name,=20namelen)=20!=3D=200)=0A=20=09=09=09continue;=0A+=09=09/*=20=
make=20sure=20to=20preserve=20case=20of=20the=20replaced=20GUC=20*/=0A+=09=
=09appendPQExpBuffer(newline,=20"%.*s=20=3D=20",=20namelen,=20where);=0A=
+=09=09if=20(guc_value_requires_quotes(guc_value))=0A+=09=09=09=
appendPQExpBuffer(newline,=20"'%s'",=20escape_quotes(guc_value));=0A+=09=09=
else=0A+=09=09=09appendPQExpBufferStr(newline,=20guc_value);=0A+=0A=20=09=
=09where=20+=3D=20namelen;=0A=20=09=09while=20(isspace((unsigned=20char)=20=
*where))=0A=20=09=09=09where++;=0A@@=20-559,7=20+561,10=20@@=20=
replace_guc_value(char=20**lines,=20const=20char=20*guc_name,=20const=20=
char=20*guc_value,=0A=20=09=09=20*=20No=20match,=20so=20append=20a=20new=20=
entry.=20=20(We=20rely=20on=20the=20bootstrap=20server=0A=20=09=09=20*=20=
to=20complain=20if=20it's=20not=20a=20valid=20GUC=20name.)=0A=20=09=09=20=
*/=0A-=09=09appendPQExpBufferChar(newline,=20'\n');=0A+=09=09if=20=
(guc_value_requires_quotes(guc_value))=0A+=09=09=09=
appendPQExpBuffer(newline,=20"%s=20=3D=20'%s'\n",=20guc_name,=20=
escape_quotes(guc_value));=0A+=09=09else=0A+=09=09=09=
appendPQExpBuffer(newline,=20"%s=20=3D=20%s\n",=20guc_name,=20=
guc_value);=0A=20=09=09lines=20=3D=20pg_realloc_array(lines,=20char=20*,=20=
i=20+=202);=0A=20=09=09lines[i++]=20=3D=20newline->data;=0A=20=09=09=
lines[i]=20=3D=20NULL;=09=09/*=20keep=20the=20array=20null-terminated=20=
*/=0Adiff=20--git=20a/src/bin/initdb/t/001_initdb.pl=20=
b/src/bin/initdb/t/001_initdb.pl=0Aindex=2003376cc0f7..413a5eca67=20=
100644=0A---=20a/src/bin/initdb/t/001_initdb.pl=0A+++=20=
b/src/bin/initdb/t/001_initdb.pl=0A@@=20-199,4=20+199,17=20@@=20=
command_fails(=0A=20command_fails([=20'initdb',=20'--no-sync',=20=
'--set',=20'foo=3Dbar',=20"$tempdir/dataX"=20],=0A=20=09'fails=20for=20=
invalid=20--set=20option');=0A=20=0A+#=20Make=20sure=20multiple=20=
invocations=20of=20-c=20parameters=20are=20added=20case=20insensitive=0A=
+command_ok(=0A+=09[=0A+=09=09'initdb',=20'-cwork_mem=3D128',=20=
'-cWork_Mem=3D256',=20'-cWORK_MEM=3D512',=0A+=09=09"$tempdir/dataY"=0A+=09=
],=0A+=09'multiple=20-c=20options=20with=20different=20case');=0A+=0A+my=20=
$conf=20=3D=20slurp_file("$tempdir/dataY/postgresql.conf");=0A+ok($conf=20=
!~=20qr/^WORK_MEM=20=3D=20/m,=20"WORK_MEM=20should=20not=20be=20=
configured");=0A+ok($conf=20!~=20qr/^Work_Mem=20=3D=20/m,=20"Work_Mem=20=
should=20not=20be=20configured");=0A+ok($conf=20=3D~=20qr/^work_mem=20=3D=20=
512/m,=20"work_mem=20should=20be=20in=20config");=0A+=0A=20=
done_testing();=0A--=20=0A2.32.1=20(Apple=20Git-133)=0A=0A=

--Apple-Mail=_9445B444-FE53-4C09-A2A4-B84EFDD728E1--