Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: Remaining dependency on setlocale()
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <450ceb6260cad30d7afdf155d991a9caafee7c0d.camel@j-davis.com>
Date: Tue, 25 Nov 2025 09:44:51 +0800
Cc: Peter Eisentraut <peter@eisentraut.org>,
 Thomas Munro <thomas.munro@gmail.com>,
 Tom Lane <tgl@sss.pgh.pa.us>,
 pgsql-hackers@postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <DDD50039-02B4-4033-8D22-B086BC448E1C@gmail.com>
References: <4c5da86af36a0d5e430eee3f60ce5e06f1b5cd34.camel@j-davis.com>
 <b19ea499a6d78c101dc55b61bfb42069b427445c.camel@j-davis.com>
 <fa04b56430fa4b5da7311c2faa113f406c6d192a.camel@j-davis.com>
 <CA+hUKGKcS-Zhi8Ud+tv2TanzXBQjH+WsLyPY9DUDXob5VoYAtg@mail.gmail.com>
 <cd3517c7-ddb8-454e-9dd5-70e3d84ff6a2@eisentraut.org>
 <d2e7d63c0cc98f425652527cf260a3e6da17d2b0.camel@j-davis.com>
 <a1396f17f462ee6561820f755caaf2d12eb9fd15.camel@j-davis.com>
 <9875f7f9-50f1-4b5d-86fc-ee8b03e8c162@eisentraut.org>
 <a8666c391dfcabe79868d95f7160eac533ace718.camel@j-davis.com>
 <6956823dd669abc64183fc91e64dc2e56e31f3ea.camel@j-davis.com>
 <ed394f839d813762ddcd65cfcab0e062f8b5e0b1.camel@j-davis.com>
 <CA+hUKGLYcbnHBXhHk_OFwZ03LE+D=cAirm7TVN8fMhT9rgcTbA@mail.gmail.com>
 <f040113cf384ada69558ec004a04a3ddb3e40a26.camel@j-davis.com>
 <CA+hUKGJxSDFcTZSGi++PTC=jM49q1oK=v7Lcdf96VQXXvLhoow@mail.gmail.com>
 <cdd20efc0f5418c597791a1e3a5abb68fa3ce35b.camel@j-davis.com>
 <d9657a6e51aa20702447bb2386b32fea6218670f.camel@j-davis.com>
 <b1ea6756-279a-4924-9146-e125af2057b2@eisentraut.org>
 <8186b28a1a39e61a0d833a4c25a8909ebbbabd48.camel@j-davis.com>
 <450ceb6260cad30d7afdf155d991a9caafee7c0d.camel@j-davis.com>
To: Jeff Davis <pgsql@j-davis.com>
Archived-At: 
 <https://www.postgresql.org/message-id/DDD50039-02B4-4033-8D22-B086BC448E1C%40gmail.com>
Precedence: bulk

Hi Jeff,

I have reviewed 0001-0004 and got a few comments:

> On Nov 25, 2025, at 07:57, Jeff Davis <pgsql@j-davis.com> wrote:
>=20
>=20
> 0001-0004: Pure refactoring patches. I intend to commit a couple of
> these soon.
>=20

1 - 0001
```
+/*
+ * Fold a character to upper case, following C/POSIX locale rules.
+ */
+static inline unsigned char
+pg_ascii_toupper(unsigned char ch)
```

I was curious why =E2=80=9Cinline=E2=80=9D is needed, then I figured out =
when I tried to build. Without =E2=80=9Cinline=E2=80=9D, compile will =
raise warnings of =E2=80=9Cunused function=E2=80=9D. So I guess it=E2=80=99=
s better to explain why =E2=80=9Cinline=E2=80=9D is used in the function =
comment, otherwise other readers may get the same confusion.

2 - 0002
```
+ * three output codepoints. See Unicode 5.18.2, "Change in Length".
```

With =E2=80=9Cchange in length=E2=80=9D, I confirmed =E2=80=9CUnicode =
5.18.2=E2=80=9D means the Unicode Standard Section 5.18.2 =
=E2=80=9CComplications for Case Mapping=E2=80=9D. Why don=E2=80=99t we =
just give the URL in the comment. =
https://www.unicode.org/versions/Unicode17.0.0/core-spec/chapter-5/#G29675=


3 - 0004
```
@@ -1282,10 +1327,18 @@ size_t
 pg_strfold(char *dst, size_t dstsize, const char *src, ssize_t srclen,
 		   pg_locale_t locale)
 {
-	if (locale->ctype->strfold)
-		return locale->ctype->strfold(dst, dstsize, src, srclen, =
locale);
-	else
-		return locale->ctype->strlower(dst, dstsize, src, =
srclen, locale);
+	if (locale->ctype =3D=3D NULL)
+	{
+		int			i;
+
+		srclen =3D (srclen >=3D 0) ? srclen : strlen(src);
+		for (i =3D 0; i < srclen && i < dstsize; i++)
+			dst[i] =3D pg_ascii_tolower(src[i]);
+		if (i < dstsize)
+			dst[i] =3D '\0';
+		return srclen;
+	}
+	return locale->ctype->strfold(dst, dstsize, src, srclen, =
locale);
 }
```

I don=E2=80=99t get this change. In old code, depending on =
locale->ctype->strfold, it calls strfold or strlower. But in this patch, =
it only calls strfold. Why? If that=E2=80=99s intentional, maybe better =
to add a comment to explain that.

4 - 0004

In pg_strfold, the ctype=3D=3DNULL fallback code is exactly the same as =
pg_strlower. I guess you intentionally to not call pg_strlower here for =
performance consideration, is that true?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/