Subject: Why is citext/regress failing on hamerkop?

From: Thomas Munro <[email-masked]>
Date: Sat, 11 May 2024 13:14:46 +1200

For example, 'i'::citext = 'İ'::citext fails to be true.

It must now be using UTF-8 (unlike, say, Drongo) and non-C ctype,
given that the test isn't skipped.  This isn't the first time that
we've noticed that Windows doesn't seem to know about İ→i (see [1]),
but I don't think anyone has explained exactly why, yet.  It could be
that it just doesn't know about that in any locale, or that it is
locale-dependent and would only do that for Turkish, the same reason
we skip the test for ICU, or ...

Either way, it seems like we'll need to skip that test on Windows if
we want hamerkop to be green.  That can probably be cribbed from
collate.windows.win1252.sql into contrib/citext/sql/citext_utf8.sql's
prelude... I just don't know how to explain it in the comment 'cause I
don't know why.

One new development in Windows-land is that the system now does
actually support UTF-8 in the runtime libraries[2].  You can set it at
a system level, or for an application at build time, or by adding
".UTF-8" to a locale name when opening the locale (apparently much
more like Unix systems, but I don't know what exactly it does).  I
wonder why we see this change now... did hamerkop have that ACP=UTF-8
setting applied on purpose, or if computers in Japan started doing
that by default instead of using Shift-JIS, or if it only started
picking UTF-8 around the time of the Meson change somehow, or the
initdb-template change.  It's a little hard to tell from the logs.

[1] https://www.postgresql.org/message-id/CAC%2BAXB10p%2BmnJ6wrAEm6jb51%2B8%3DBfYzD%3Dw6ftHRbMjMuSFN3kQ%40mail.gmail.com
[2] https://learn.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-code-page

---

From: Thomas Munro <[email-masked]>
Date: Sun, 12 May 2024 11:31:09 +1200

> prelude... I just don't know how to explain it in the comment 'cause I

Here's a minimal patch like that.

I don't think it's worth back-patching.  Only 15 and 16 could possibly
be affected, I think, because the test wasn't enabled before that.  I
think this is all just a late-appearing blow-up predicted by the
commit that enabled it:

commit c2e8bd27519f47ff56987b30eb34a01969b9a9e8
Author: Tom Lane <[email-masked]>
Date:   Wed Jan 5 13:30:07 2022 -0500

    Enable routine running of citext's UTF8-specific test cases.

    These test cases have been commented out since citext was invented,
    because at the time we had no nice way to deal with tests that
    have restrictions such as requiring UTF8 encoding.  But now we do
    have a convention for that, ie put them into a separate test file
    with an early-exit path.  So let's enable these tests to run when
    their prerequisites are satisfied.

    (We may have to tighten the prerequisites beyond the "encoding = UTF8
    and locale != C" checks made here.  But let's put it on the buildfarm
    and see what blows up.)

Hamerkop is already green on the 15 and 16 branches, apparently
because it's using the pre-meson test stuff that I guess just didn't
run the relevant test.  In other words, nobody would notice the
difference anyway, and a master-only fix would be enough to end this
44-day red streak.
From 1f0e1dc21d4055a0e5109bac39999b290508e2d8 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email-masked]>
Date: Sun, 12 May 2024 10:20:06 +1200
Subject: [PATCH] Skip the citext_utf8 test on Windows.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On other Windows build farm animals it is already skipped because they
don't use UTF-8 encoding.  On "hamerkop", UTF-8 is used, and then the
test fails.

It is not clear to me (a non-Windows person looking only at buildfarm
evidence) whether Windows is less sophisticated than other OSes and
doesn't know how to downcase Turkish İ with the standard Unicode
database, or if it is more sophisticated than other systems and uses
locale-specific behavior like ICU does.

Whichever the reason, the result is the same: we need to skip the test
on Windows, just as we already do for ICU, at least until a
Windows-savvy developer comes up with a better idea.  The technique for
detecting the OS is borrowed from collate.windows.win1252.sql.

Discussion: https://postgr.es/m/CA%2BhUKGJ1LeC3aE2qQYTK95rFVON3ZVoTQpTKJqxkHdtEyawH4A%40mail.gmail.com
---
 contrib/citext/expected/citext_utf8.out   | 3 +++
 contrib/citext/expected/citext_utf8_1.out | 3 +++
 contrib/citext/sql/citext_utf8.sql        | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/contrib/citext/expected/citext_utf8.out b/contrib/citext/expected/citext_utf8.out
index 5d988dcd485..19538db674e 100644
--- a/contrib/citext/expected/citext_utf8.out
+++ b/contrib/citext/expected/citext_utf8.out
@@ -6,8 +6,11 @@
  * Turkish dotted I is not correct for many ICU locales. citext always
  * uses the default collation, so it's not easy to restrict the test
  * to the "tr-TR-x-icu" collation where it will succeed.
+ *
+ * Also disable for Windows.  It fails similarly, at least in some locales.
  */
 SELECT getdatabaseencoding() <> 'UTF8' OR
+       version() ~ '(Visual C\+\+|mingw32|windows)' OR
        (SELECT (datlocprovider = 'c' AND datctype = 'C') OR datlocprovider = 'i'
         FROM pg_database
         WHERE datname=current_database())
diff --git a/contrib/citext/expected/citext_utf8_1.out b/contrib/citext/expected/citext_utf8_1.out
index 7065a5da190..874ec8519e1 100644
--- a/contrib/citext/expected/citext_utf8_1.out
+++ b/contrib/citext/expected/citext_utf8_1.out
@@ -6,8 +6,11 @@
  * Turkish dotted I is not correct for many ICU locales. citext always
  * uses the default collation, so it's not easy to restrict the test
  * to the "tr-TR-x-icu" collation where it will succeed.
+ *
+ * Also disable for Windows.  It fails similarly, at least in some locales.
  */
 SELECT getdatabaseencoding() <> 'UTF8' OR
+       version() ~ '(Visual C\+\+|mingw32|windows)' OR
        (SELECT (datlocprovider = 'c' AND datctype = 'C') OR datlocprovider = 'i'
         FROM pg_database
         WHERE datname=current_database())
diff --git a/contrib/citext/sql/citext_utf8.sql b/contrib/citext/sql/citext_utf8.sql
index 34b232d64e2..ba283320797 100644
--- a/contrib/citext/sql/citext_utf8.sql
+++ b/contrib/citext/sql/citext_utf8.sql
@@ -6,9 +6,12 @@
  * Turkish dotted I is not correct for many ICU locales. citext always
  * uses the default collation, so it's not easy to restrict the test
  * to the "tr-TR-x-icu" collation where it will succeed.
+ *
+ * Also disable for Windows.  It fails similarly, at least in some locales.
  */
 
 SELECT getdatabaseencoding() <> 'UTF8' OR
+       version() ~ '(Visual C\+\+|mingw32|windows)' OR
        (SELECT (datlocprovider = 'c' AND datctype = 'C') OR datlocprovider = 'i'
         FROM pg_database
         WHERE datname=current_database())
-- 
2.44.0

---

From: Oleg Tselebrovskiy <[email-masked]>
Date: Thu, 01 Aug 2024 20:37:12 +0700

> difference anyway, and a master-only fix would be enough to end this

Sorry for necroposting, but in our automated testing system we have
found some fails of this test. The most recent one was a couple of
days ago (see attached files) on PostgreSQL 15.7. Also I've reported
this bug some time ago [1], but provided an example only for
PostgreSQL 17. Back then the bug was actually found on 15 or 16
branches (no logs remain from couple of months back), but i wanted
to show that it was reproducible on 17.

I would appreciate if you would backpatch this change to 15 and 16
branches.

[1] 
https://www.postgresql.org/message-id/6885a0b52d06f7e5910d2b6276bbb4e8%40postgrespro.ru

Oleg Tselebrovskiy, Postgres Pro

diff -w -U3 C:/gr-builds/TaKFe3FF/2/pgpro-dev/postgrespro/contrib/citext/expected/citext_utf8.out C:/gr-builds/TaKFe3FF/2/pgpro-dev/postgrespro/contrib/citext/results/citext_utf8.out
--- C:/gr-builds/TaKFe3FF/2/pgpro-dev/postgrespro/contrib/citext/expected/citext_utf8.out	2024-07-29 13:53:45.259126600 +0300
+++ C:/gr-builds/TaKFe3FF/2/pgpro-dev/postgrespro/contrib/citext/results/citext_utf8.out	2024-07-29 14:43:38.772857200 +0300
@@ -54,7 +54,7 @@
 SELECT 'i'::citext = 'İ'::citext AS t;
  t 
 ---
- t
+ f
 (1 row)
 
 -- Regression.

---

From: Thomas Munro <[email-masked]>
Date: Fri, 2 Aug 2024 10:54:02 +1200

> I would appreciate if you would backpatch this change to 15 and 16

Done (e52a44b8, 91f498fd).

Any elucidation on how and why Windows machines have started using
UTF-8 would be welcome.