Subject: Re: tiny step toward threading: reduce dependence on setlocale()

From: Jeff Davis <[email-masked]>
Date: Tue, 06 Aug 2024 14:40:17 -0700

On Tue, 2024-07-30 at 12:13 -0700, Jeff Davis wrote:
> I found a couple issues with the later patches:
> 
> * There was still some confusion about the default collation vs.
> datcollate/datctype for callers of wchar2char() and char2wchar()
> (those
> functions only work for libc). I introduced a new pg_locale_t
> structure
> to represent datcollate/datctype regardless of datlocprovider to
> solve
> this.

I didn't quite like the API I introduced in 0001, so I skipped 0001.

For 0002 I left char2wchar() and wchar2char() as-is, where they expect
libc and accept a NULL pg_locale_t. I committed the rest of 0002.

> * Another loose end relying on setlocale(): in selfuncs.c, there's
> still a call directly to strxfrm(), which depends on setlocale(). I
> changed this to lookup the collation and then use pg_strxfrm(). That
> should improve histogram selectivity estimates because it uses the
> correct provider, rather than relying on setlocale(), right?

Committed 0003.

With these changes, collations are no longer dependent on the
environment locale (setlocale()) at all for either collation behavior
(ORDER BY) or ctype behavior (LOWER(), etc.).

Additionally, unless I missed something, nothing in the server is
dependent on LC_COLLATE at all.

There are still some things that depend on setlocale() in one way or
another:

  - char2wchar() & wchar2char()
  - ts_locale.c
  - various places that depend on LC_CTYPE unrelated to the collation
infrastructure
  - things that depend on other locale settings, like LC_NUMERIC

We can address those as part of a separate thread. I'll count this as
committed.

Regards,
	Jeff Davis

---

From: Peter Eisentraut <[email-masked]>
Date: Wed, 7 Aug 2024 22:44:25 +0200

> We can address those as part of a separate thread. I'll count this as

I have a couple of small follow-up patches for this.

First, in like.c, SB_lower_char() now reads:

static char
SB_lower_char(unsigned char c, pg_locale_t locale, bool locale_is_c)
{
     if (locale_is_c)
         return pg_ascii_tolower(c);
     else if (locale)
         return tolower_l(c, locale->info.lt);
     else
         return pg_tolower(c);
}

But after this patch set, locale cannot be NULL anymore, so the third 
branch is obsolete.

(Now that I look at it, pg_tolower() has some short-circuiting for ASCII 
letters, so it would not handle Turkish-i correctly if that had been the 
global locale.  By removing the use of pg_tolower(), we fix that issue 
in passing.)

Second, there are a number of functions in like.c like the above that 
take separate arguments like pg_locale_t locale, bool locale_is_c. 
Because pg_locale_t now contains the locale_is_c information, these can 
be combined.

From 7de0b6c3069c5fbef7729b7a86e40b75dfb616fd Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email-masked]>
Date: Wed, 7 Aug 2024 22:24:39 +0200
Subject: [PATCH 1/2] Remove dead code

After e9931bfb751, the locale argument of SB_lower_char() is never not
NULL, so the branch that deals with NULL can be removed.
---
 src/backend/utils/adt/like.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/backend/utils/adt/like.c b/src/backend/utils/adt/like.c
index 0ecc96d48e5..bb4d73fb22e 100644
--- a/src/backend/utils/adt/like.c
+++ b/src/backend/utils/adt/like.c
@@ -95,10 +95,8 @@ SB_lower_char(unsigned char c, pg_locale_t locale, bool locale_is_c)
 {
 	if (locale_is_c)
 		return pg_ascii_tolower(c);
-	else if (locale)
-		return tolower_l(c, locale->info.lt);
 	else
-		return pg_tolower(c);
+		return tolower_l(c, locale->info.lt);
 }

-- 
2.46.0

From 5d3b595fc08c3a013b5ebe532b3922bf9db7d963 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email-masked]>
Date: Wed, 7 Aug 2024 22:30:30 +0200
Subject: [PATCH 2/2] Remove separate locale_is_c arguments

Since e9931bfb751, ctype_is_c is part of pg_locale_t.  Some functions
passed a pg_locale_t and a bool argument separately.  This can now be
combined into one argument.
---
 src/backend/utils/adt/like.c       | 30 +++++++++++++++---------------
 src/backend/utils/adt/like_match.c |  6 ++----
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/src/backend/utils/adt/like.c b/src/backend/utils/adt/like.c
index bb4d73fb22e..a3a1178887c 100644
--- a/src/backend/utils/adt/like.c
+++ b/src/backend/utils/adt/like.c
@@ -33,18 +33,18 @@

 static int	SB_MatchText(const char *t, int tlen, const char *p, int plen,
-						 pg_locale_t locale, bool locale_is_c);
+						 pg_locale_t locale);
 static text *SB_do_like_escape(text *pat, text *esc);
 
 static int	MB_MatchText(const char *t, int tlen, const char *p, int plen,
-						 pg_locale_t locale, bool locale_is_c);
+						 pg_locale_t locale);
 static text *MB_do_like_escape(text *pat, text *esc);
 
 static int	UTF8_MatchText(const char *t, int tlen, const char *p, int plen,
-						   pg_locale_t locale, bool locale_is_c);
+						   pg_locale_t locale);
 
 static int	SB_IMatchText(const char *t, int tlen, const char *p, int plen,
-						  pg_locale_t locale, bool locale_is_c);
+						  pg_locale_t locale);
 
 static int	GenericMatchText(const char *s, int slen, const char *p, int plen, Oid collation);
 static int	Generic_Text_IC_like(text *str, text *pat, Oid collation);
@@ -91,9 +91,9 @@ wchareq(const char *p1, const char *p2)
  * fold-on-the-fly processing, however.
  */
 static char
-SB_lower_char(unsigned char c, pg_locale_t locale, bool locale_is_c)
+SB_lower_char(unsigned char c, pg_locale_t locale)
 {
-	if (locale_is_c)
+	if (locale->ctype_is_c)
 		return pg_ascii_tolower(c);
 	else
 		return tolower_l(c, locale->info.lt);
@@ -129,7 +129,7 @@ SB_lower_char(unsigned char c, pg_locale_t locale, bool locale_is_c)
 #include "like_match.c"
 
 /* setup to compile like_match.c for single byte case insensitive matches */
-#define MATCH_LOWER(t) SB_lower_char((unsigned char) (t), locale, locale_is_c)
+#define MATCH_LOWER(t) SB_lower_char((unsigned char) (t), locale)
 #define NextChar(p, plen) NextByte((p), (plen))
 #define MatchText SB_IMatchText
 
@@ -158,11 +158,11 @@ GenericMatchText(const char *s, int slen, const char *p, int plen, Oid collation
 	}
 
 	if (pg_database_encoding_max_length() == 1)
-		return SB_MatchText(s, slen, p, plen, 0, true);
+		return SB_MatchText(s, slen, p, plen, 0);
 	else if (GetDatabaseEncoding() == PG_UTF8)
-		return UTF8_MatchText(s, slen, p, plen, 0, true);
+		return UTF8_MatchText(s, slen, p, plen, 0);
 	else
-		return MB_MatchText(s, slen, p, plen, 0, true);
+		return MB_MatchText(s, slen, p, plen, 0);
 }
 
 static inline int
@@ -212,9 +212,9 @@ Generic_Text_IC_like(text *str, text *pat, Oid collation)
 		s = VARDATA_ANY(str);
 		slen = VARSIZE_ANY_EXHDR(str);
 		if (GetDatabaseEncoding() == PG_UTF8)
-			return UTF8_MatchText(s, slen, p, plen, 0, true);
+			return UTF8_MatchText(s, slen, p, plen, 0);
 		else
-			return MB_MatchText(s, slen, p, plen, 0, true);
+			return MB_MatchText(s, slen, p, plen, 0);
 	}
 	else
 	{
@@ -222,7 +222,7 @@ Generic_Text_IC_like(text *str, text *pat, Oid collation)
 		plen = VARSIZE_ANY_EXHDR(pat);
 		s = VARDATA_ANY(str);
 		slen = VARSIZE_ANY_EXHDR(str);
-		return SB_IMatchText(s, slen, p, plen, locale, locale->ctype_is_c);
+		return SB_IMatchText(s, slen, p, plen, locale);
 	}
 }
 
@@ -330,7 +330,7 @@ bytealike(PG_FUNCTION_ARGS)
 	p = VARDATA_ANY(pat);
 	plen = VARSIZE_ANY_EXHDR(pat);
 
-	result = (SB_MatchText(s, slen, p, plen, 0, true) == LIKE_TRUE);
+	result = (SB_MatchText(s, slen, p, plen, 0) == LIKE_TRUE);
 
 	PG_RETURN_BOOL(result);
 }
@@ -351,7 +351,7 @@ byteanlike(PG_FUNCTION_ARGS)
 	p = VARDATA_ANY(pat);
 	plen = VARSIZE_ANY_EXHDR(pat);
 
-	result = (SB_MatchText(s, slen, p, plen, 0, true) != LIKE_TRUE);
+	result = (SB_MatchText(s, slen, p, plen, 0) != LIKE_TRUE);
 
 	PG_RETURN_BOOL(result);
 }
diff --git a/src/backend/utils/adt/like_match.c b/src/backend/utils/adt/like_match.c
index f2990edff7e..928a1e99fc5 100644
--- a/src/backend/utils/adt/like_match.c
+++ b/src/backend/utils/adt/like_match.c
@@ -77,8 +77,7 @@
 #endif
 
 static int
-MatchText(const char *t, int tlen, const char *p, int plen,
-		  pg_locale_t locale, bool locale_is_c)
+MatchText(const char *t, int tlen, const char *p, int plen, pg_locale_t locale)
 {
 	/* Fast path for match-everything pattern */
 	if (plen == 1 && *p == '%')
@@ -175,8 +174,7 @@ MatchText(const char *t, int tlen, const char *p, int plen,
 			{
 				if (GETCHAR(*t) == firstpat)
 				{
-					int			matched = MatchText(t, tlen, p, plen,
-													locale, locale_is_c);
+					int			matched = MatchText(t, tlen, p, plen, locale);
 
 					if (matched != LIKE_FALSE)
 						return matched; /* TRUE or ABORT */
-- 
2.46.0

---

From: Jeff Davis <[email-masked]>
Date: Thu, 08 Aug 2024 13:00:41 -0700

> But after this patch set, locale cannot be NULL anymore, so the third

...

> Because pg_locale_t now contains the locale_is_c information, these

I believe these patches are correct, but the reasoning is fairly
complex:

1. Some MatchText variants are called with 0 for locale. But that's OK
because ...

2. A MatchText variant only cares about the locale if MATCH_LOWER(t) is
defined, and ...

3. Only one variant, SB_IMatchText() defines MATCH_LOWER(), and ...

4. SB_IMatchText() is called with a non-zero locale.

All of these are a bit confusing to follow because it's generated code.
#2 is particularly non-obvious, because "locale" is not even an
argument of the MATCH_LOWER(t) or GETCHAR(t) macros, it's taken
implicitly from the outer scope.

I don't think your patches cause this confusion, but is there a way you
can clarify some of this along the way?

Regards,
	Jeff Davis

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 8 Aug 2024 23:09:53 +0200

> global locale.  By removing the use of pg_tolower(), we fix that issue 

It occurred to me that this issue also surfaces in a more prominent 
place.  These arguably-wrong pg_tolower() and pg_toupper() calls were 
also used by the normal SQL lower() and upper() functions before commit 
e9931bfb751 if you used a single byte encoding.

For example, in PG17, multi-byte encoding:

initdb --locale=tr_TR.utf8

select upper('hij'); --> HİJ

PG17, single-byte encoding:

initdb --locale=tr_TR  # uses LATIN5

select upper('hij'); --> HIJ

With current master, after commit e9931bfb751, you get the first result 
in both cases.

So this could break indexes across pg_upgrade in such configurations.

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 8 Aug 2024 23:18:33 +0200

> can clarify some of this along the way?

Yes, this is also my analysis.  The patch in 
<https://www.postgresql.org/message-id/flat/[email-masked]> 
would replace passing 0 with an actual locale object.  The changes in 
GenericMatchText() could also be applied independently so that we'd 
always pass in a non-zero locale value, even if it would not be used in 
some cases.  I need to update that patch to cover your latest changes. 
I'll see if I can propose something here that looks a bit nicer.

---

From: Jeff Davis <[email-masked]>
Date: Thu, 08 Aug 2024 15:38:09 -0700

> So this could break indexes across pg_upgrade in such configurations.

Good observation. Is there any action we should take here, or just add
that to the release notes for 18?

Regards,
	Jeff Davis

---

From: Tom Lane <[email-masked]>
Date: Sun, 11 Aug 2024 12:33:30 -0400

> We can address those as part of a separate thread. I'll count this as

Coverity has a nit about this:

*** CID 1616189:  Null pointer dereferences  (REVERSE_INULL)
/srv/coverity/git/pgsql-git/postgresql/src/backend/utils/adt/like.c: 206 in Generic_Text_IC_like()
200     	 * on the pattern and text, but instead call SB_lower_char on each
201     	 * character.  In the multi-byte case we don't have much choice :-(. Also,
202     	 * ICU does not support single-character case folding, so we go the long
203     	 * way.
204     	 */
205     
>>>     CID 1616189:  Null pointer dereferences  (REVERSE_INULL)
>>>     Null-checking "locale" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
206     	if (pg_database_encoding_max_length() > 1 || (locale && locale->provider == COLLPROVIDER_ICU))
207     	{
208     		pat = DatumGetTextPP(DirectFunctionCall1Coll(lower, collation,
209     													 PointerGetDatum(pat)));
210     		p = VARDATA_ANY(pat);
211     		plen = VARSIZE_ANY_EXHDR(pat);

I assume it would now be okay to take out "locale &&" here?

			regards, tom lane

---

From: Peter Eisentraut <[email-masked]>
Date: Mon, 12 Aug 2024 09:00:11 +0200

> I assume it would now be okay to take out "locale &&" here?

Correct, that check is no longer necessary.

---

From: Jeff Davis <[email-masked]>
Date: Mon, 12 Aug 2024 12:47:58 -0700

> Correct, that check is no longer necessary.

Thank you, fixed.

Regards,
	Jeff Davis

---

From: Peter Eisentraut <[email-masked]>
Date: Tue, 13 Aug 2024 17:11:50 +0200

I'm wondering if after this patch series, lc_collate_is_c() and 
lc_ctype_is_c() are still useful.

They used to be completely separate from pg_newlocale_from_collation(), 
but now they are just mostly a thin wrapper around it.  Except there is 
some hardcoded handling of C_COLLATION_OID and POSIX_COLLATION_OID.  Do 
we care about that?

In many places, the notational and structural complexity would be 
significantly improved if we changed code like

     if (pg_collate_is_c(colloid))
     {
         ...
     }
     else
     {
         pg_locale_t locale = pg_newlocale_from_collation(colloid);

         if (locale->provider == ...)
         {
             ...
     }

to more like

     pg_locale_t locale = pg_newlocale_from_collation(colloid);

     if (locale->collate_is_c)
     {
         ...
     }
     else if (locale->provider == ...)
         ...
     }
     ...

However, it's not clear whether the hardcoded handling of some 
collations is needed for performance parity or perhaps some 
bootstrapping reasons.  It would be useful to get that cleared up. 
Thoughts?

---

From: Jeff Davis <[email-masked]>
Date: Tue, 13 Aug 2024 10:56:07 -0700

...

> bootstrapping reasons.  It would be useful to get that cleared up. 

There's at least one place where we expect lc_collate_is_c() to work
without catalog access at all: libpq/hba.c uses regexes with
C_COLLATION_OID.

But I don't think that's a major problem -- we can just move the
hardcoded test into pg_newlocale_from_collation() and return a
predefined struct with collate_is_c/ctype_is_c already set.

+1.

Regards,
	Jeff Davis

---

From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 01:31:11 +0200

> predefined struct with collate_is_c/ctype_is_c already set.

I tried that out but thought it felt cleaner to do the hardcoding in 
pg_set_regex_collation(). What do you think?

I have attached patches removing lc_collate_is_c() and lc_ctype_is_c(). 
I have not checked if there are any performance regressions when using 
the C and POSIX locales but remove these special cases makes the code a 
lot cleaner in my book.

I also attach some other clean up patches I did while touching this code.

0001-Remove-lc_collate_is_c.patch

Removes lc_collate_is_c().

0002-Remove-lc_ctype_is_c.patch

Removes lc_ctype_is_c() and POSIX_COLLATION_OID which is no longer 
necessary.

0003-Remove-dubious-check-against-default-locale.patch

This patch removes a check against DEFAULT_COLLATION_OID which I thought 
looked really dubious. Shouldn't this just be a simple check for if the 
locale is deterministic? Since we know we have a valid locale that 
should be enough, right?

0004-Do-not-check-both-for-collate_is_c-and-deterministic.patch

It is redundant to check both for "collation_is_c && deterministic", right?

0005-Remove-pg_collate_deterministic-and-check-field-dire.patch

Since after my patches we look a lot directly at the collation_is_c and 
ctype_is_c fields I think the thin wrapper around the deterministic 
field makes it seem like there is more to it so I suggest that we should 
just remove it.

0006-Slightly-refactor-varstr_sortsupport-to-improve-read.patch

Small refactor to make a hard to read function a bit easier to read.

Andreas

From 99ffa0b76ce810846af09f4430903dd59ed06366 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:37:23 +0200
Subject: [PATCH 1/6] Remove lc_collate_is_c()

Instead always look up the collation and check to collate_is_c field.

TODO: Check for performance regression?
---
 src/backend/access/spgist/spgtextproc.c |  2 +-
 src/backend/commands/collationcmds.c    |  8 ++-----
 src/backend/utils/adt/like_support.c    |  4 ++--
 src/backend/utils/adt/pg_locale.c       | 27 ---------------------
 src/backend/utils/adt/selfuncs.c        |  6 +++--
 src/backend/utils/adt/varchar.c         | 20 +++++-----------
 src/backend/utils/adt/varlena.c         | 32 ++++++++++++-------------
 src/include/utils/pg_locale.h           |  1 -
 8 files changed, 30 insertions(+), 70 deletions(-)

diff --git a/src/backend/access/spgist/spgtextproc.c b/src/backend/access/spgist/spgtextproc.c
index 3f08d330b6..d5237a68b5 100644
--- a/src/backend/access/spgist/spgtextproc.c
+++ b/src/backend/access/spgist/spgtextproc.c
@@ -427,7 +427,7 @@ spg_text_inner_consistent(PG_FUNCTION_ARGS)
 {
 	spgInnerConsistentIn *in = (spgInnerConsistentIn *) PG_GETARG_POINTER(0);
 	spgInnerConsistentOut *out = (spgInnerConsistentOut *) PG_GETARG_POINTER(1);
-	bool		collate_is_c = lc_collate_is_c(PG_GET_COLLATION());
+	bool		collate_is_c = pg_newlocale_from_collation(PG_GET_COLLATION())->collate_is_c;
 	text	   *reconstructedValue;
 	text	   *reconstrText;
 	int			maxReconstrLen;
diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c
index 63ef9a0841..53b6a479aa 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -377,13 +377,9 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e
 	if (!OidIsValid(newoid))
 		return InvalidObjectAddress;
 
-	/*
-	 * Check that the locales can be loaded.  NB: pg_newlocale_from_collation
-	 * is only supposed to be called on non-C-equivalent locales.
-	 */
+	/* Check that the locales can be loaded. */
 	CommandCounterIncrement();
-	if (!lc_collate_is_c(newoid) || !lc_ctype_is_c(newoid))
-		(void) pg_newlocale_from_collation(newoid);
+	(void) pg_newlocale_from_collation(newoid);
 
 	ObjectAddressSet(address, CollationRelationId, newoid);
 
diff --git a/src/backend/utils/adt/like_support.c b/src/backend/utils/adt/like_support.c
index 2635050861..a579fa0157 100644
--- a/src/backend/utils/adt/like_support.c
+++ b/src/backend/utils/adt/like_support.c
@@ -433,7 +433,7 @@ match_pattern_prefix(Node *leftop,
 	 * collation.
 	 */
 	if (collation_aware &&
-		!lc_collate_is_c(indexcollation))
+		!pg_newlocale_from_collation(indexcollation)->collate_is_c)
 		return NIL;
 
 	/*
@@ -1603,7 +1603,7 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation)
 		else
 			workstr = TextDatumGetCString(str_const->constvalue);
 		len = strlen(workstr);
-		if (lc_collate_is_c(collation) || len == 0)
+		if (pg_newlocale_from_collation(collation)->collate_is_c || len == 0)
 			cmpstr = str_const->constvalue;
 		else
 		{
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index cd3661e727..7512ac055e 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1258,33 +1258,6 @@ lookup_collation_cache(Oid collation)
 	return cache_entry;
 }
 
-
-/*
- * Detect whether collation's LC_COLLATE property is C
- */
-bool
-lc_collate_is_c(Oid collation)
-{
-	/*
-	 * If we're asked about "collation 0", return false, so that the code will
-	 * go into the non-C path and report that the collation is bogus.
-	 */
-	if (!OidIsValid(collation))
-		return false;
-
-	/*
-	 * If we're asked about the built-in C/POSIX collations, we know that.
-	 */
-	if (collation == C_COLLATION_OID ||
-		collation == POSIX_COLLATION_OID)
-		return true;
-
-	/*
-	 * Otherwise, we have to consult pg_collation, but we cache that.
-	 */
-	return pg_newlocale_from_collation(collation)->collate_is_c;
-}
-
 /*
  * Detect whether collation's LC_CTYPE property is C
  */
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index bf42393bec..03d7fb5f48 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -4646,6 +4646,7 @@ static char *
 convert_string_datum(Datum value, Oid typid, Oid collid, bool *failure)
 {
 	char	   *val;
+	pg_locale_t mylocale;
 
 	switch (typid)
 	{
@@ -4671,9 +4672,10 @@ convert_string_datum(Datum value, Oid typid, Oid collid, bool *failure)
 			return NULL;
 	}
 
-	if (!lc_collate_is_c(collid))
+	mylocale = pg_newlocale_from_collation(collid);
+
+	if (!mylocale->collate_is_c)
 	{
-		pg_locale_t mylocale = pg_newlocale_from_collation(collid);
 		char	   *xfrmstr;
 		size_t		xfrmlen;
 		size_t		xfrmlen2 PG_USED_FOR_ASSERTS_ONLY;
diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c
index 0c219dcc77..d2d1c5709d 100644
--- a/src/backend/utils/adt/varchar.c
+++ b/src/backend/utils/adt/varchar.c
@@ -748,20 +748,16 @@ bpchareq(PG_FUNCTION_ARGS)
 				len2;
 	bool		result;
 	Oid			collid = PG_GET_COLLATION();
-	bool		locale_is_c = false;
-	pg_locale_t mylocale = 0;
+	pg_locale_t mylocale;
 
 	check_collation_set(collid);
 
 	len1 = bcTruelen(arg1);
 	len2 = bcTruelen(arg2);
 
-	if (lc_collate_is_c(collid))
-		locale_is_c = true;
-	else
-		mylocale = pg_newlocale_from_collation(collid);
+	mylocale = pg_newlocale_from_collation(collid);
 
-	if (locale_is_c || pg_locale_deterministic(mylocale))
+	if (mylocale->collate_is_c || pg_locale_deterministic(mylocale))
 	{
 		/*
 		 * Since we only care about equality or not-equality, we can avoid all
@@ -793,20 +789,16 @@ bpcharne(PG_FUNCTION_ARGS)
 				len2;
 	bool		result;
 	Oid			collid = PG_GET_COLLATION();
-	bool		locale_is_c = false;
-	pg_locale_t mylocale = 0;
+	pg_locale_t mylocale;
 
 	check_collation_set(collid);
 
 	len1 = bcTruelen(arg1);
 	len2 = bcTruelen(arg2);
 
-	if (lc_collate_is_c(collid))
-		locale_is_c = true;
-	else
-		mylocale = pg_newlocale_from_collation(collid);
+	mylocale = pg_newlocale_from_collation(collid);
 
-	if (locale_is_c || pg_locale_deterministic(mylocale))
+	if (mylocale->collate_is_c || pg_locale_deterministic(mylocale))
 	{
 		/*
 		 * Since we only care about equality or not-equality, we can avoid all
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 52ab8c43c6..81343bd0e1 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -1538,16 +1538,19 @@ int
 varstr_cmp(const char *arg1, int len1, const char *arg2, int len2, Oid collid)
 {
 	int			result;
+	pg_locale_t mylocale;
 
 	check_collation_set(collid);
 
+	mylocale = pg_newlocale_from_collation(collid);
+
 	/*
 	 * Unfortunately, there is no strncoll(), so in the non-C locale case we
 	 * have to do some memory copying.  This turns out to be significantly
 	 * slower, so we optimize the case where LC_COLLATE is C.  We also try to
 	 * optimize relatively-short strings by avoiding palloc/pfree overhead.
 	 */
-	if (lc_collate_is_c(collid))
+	if (mylocale->collate_is_c)
 	{
 		result = memcmp(arg1, arg2, Min(len1, len2));
 		if ((result == 0) && (len1 != len2))
@@ -1555,10 +1558,6 @@ varstr_cmp(const char *arg1, int len1, const char *arg2, int len2, Oid collid)
 	}
 	else
 	{
-		pg_locale_t mylocale;
-
-		mylocale = pg_newlocale_from_collation(collid);
-
 		/*
 		 * memcmp() can't tell us which of two unequal strings sorts first,
 		 * but it's a cheap way to tell if they're equal.  Testing shows that
@@ -1865,10 +1864,12 @@ varstr_sortsupport(SortSupport ssup, Oid typid, Oid collid)
 	bool		abbreviate = ssup->abbreviate;
 	bool		collate_c = false;
 	VarStringSortSupport *sss;
-	pg_locale_t locale = 0;
+	pg_locale_t locale;
 
 	check_collation_set(collid);
 
+	locale = pg_newlocale_from_collation(collid);
+
 	/*
 	 * If possible, set ssup->comparator to a function which can be used to
 	 * directly compare two datums.  If we can do this, we'll avoid the
@@ -1882,7 +1883,7 @@ varstr_sortsupport(SortSupport ssup, Oid typid, Oid collid)
 	 * varstrfastcmp_c, bpcharfastcmp_c, or namefastcmp_c, all of which use
 	 * memcmp() rather than strcoll().
 	 */
-	if (lc_collate_is_c(collid))
+	if (locale->collate_is_c)
 	{
 		if (typid == BPCHAROID)
 			ssup->comparator = bpcharfastcmp_c;
@@ -1899,13 +1900,6 @@ varstr_sortsupport(SortSupport ssup, Oid typid, Oid collid)
 	}
 	else
 	{
-		/*
-		 * We need a collation-sensitive comparison.  To make things faster,
-		 * we'll figure out the collation based on the locale id and cache the
-		 * result.
-		 */
-		locale = pg_newlocale_from_collation(collid);
-
 		/*
 		 * We use varlenafastcmp_locale except for type NAME.
 		 */
@@ -1956,7 +1950,8 @@ varstr_sortsupport(SortSupport ssup, Oid typid, Oid collid)
 		sss->last_len2 = -1;
 		/* Initialize */
 		sss->last_returned = 0;
-		sss->locale = locale;
+		if (!collate_c)
+			sss->locale = locale;
 
 		/*
 		 * To avoid somehow confusing a strxfrm() blob and an original string,
@@ -2546,12 +2541,15 @@ btvarstrequalimage(PG_FUNCTION_ARGS)
 {
 	/* Oid		opcintype = PG_GETARG_OID(0); */
 	Oid			collid = PG_GET_COLLATION();
+	pg_locale_t locale;
 
 	check_collation_set(collid);
 
-	if (lc_collate_is_c(collid) ||
+	locale = pg_newlocale_from_collation(collid);
+
+	if (locale->collate_is_c ||
 		collid == DEFAULT_COLLATION_OID ||
-		get_collation_isdeterministic(collid))
+		pg_locale_deterministic(locale))
 		PG_RETURN_BOOL(true);
 	else
 		PG_RETURN_BOOL(false);
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index f41d33975b..8ec24437f4 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -54,7 +54,6 @@ extern PGDLLIMPORT bool database_ctype_is_c;
 extern bool check_locale(int category, const char *locale, char **canonname);
 extern char *pg_perm_setlocale(int category, const char *locale);
 
-extern bool lc_collate_is_c(Oid collation);
 extern bool lc_ctype_is_c(Oid collation);
 
 /*
-- 
2.43.0

From c7486013aca89bab8178cb7ca0944c52d5805820 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:37:48 +0200
Subject: [PATCH 2/6] Remove lc_ctype_is_c()

Instead always fetch the locale and look at the ctype_is_c field.

Since regular expressions are used by pg_hba we need to hardcode
the logic for regular expression using C_COLLATE_OID.
---
 src/backend/regex/regc_pg_locale.c   | 20 ++++++++++++++------
 src/backend/utils/adt/formatting.c   | 27 ++++++++++++---------------
 src/backend/utils/adt/like.c         |  2 +-
 src/backend/utils/adt/like_support.c | 15 +++++----------
 src/backend/utils/adt/pg_locale.c    | 26 --------------------------
 src/include/catalog/pg_collation.dat |  3 +--
 src/include/utils/pg_locale.h        |  2 --
 7 files changed, 33 insertions(+), 62 deletions(-)

diff --git a/src/backend/regex/regc_pg_locale.c b/src/backend/regex/regc_pg_locale.c
index 947d73f3e0..21f640a7e1 100644
--- a/src/backend/regex/regc_pg_locale.c
+++ b/src/backend/regex/regc_pg_locale.c
@@ -243,29 +243,36 @@ pg_set_regex_collation(Oid collation)
 				 errhint("Use the COLLATE clause to set the collation explicitly.")));
 	}
 
-	if (lc_ctype_is_c(collation))
+	if (collation == C_COLLATION_OID)
 	{
-		/* C/POSIX collations use this path regardless of database encoding */
+		/* Allow using regex in C locale without catalog */
 		pg_regex_strategy = PG_REGEX_LOCALE_C;
 		pg_regex_locale = 0;
 		pg_regex_collation = C_COLLATION_OID;
 	}
 	else
 	{
-		pg_regex_locale = pg_newlocale_from_collation(collation);
+		pg_locale_t locale = pg_newlocale_from_collation(collation);
 
-		if (!pg_locale_deterministic(pg_regex_locale))
+		if (!pg_locale_deterministic(locale))
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("nondeterministic collations are not supported for regular expressions")));
 
-		if (pg_regex_locale->provider == COLLPROVIDER_BUILTIN)
+		if (locale->ctype_is_c)
+		{
+			/* C/POSIX collations use this path regardless of database encoding */
+			pg_regex_strategy = PG_REGEX_LOCALE_C;
+			locale = 0;
+			collation = C_COLLATION_OID;
+		}
+		else if (locale->provider == COLLPROVIDER_BUILTIN)
 		{
 			Assert(GetDatabaseEncoding() == PG_UTF8);
 			pg_regex_strategy = PG_REGEX_BUILTIN;
 		}
 #ifdef USE_ICU
-		else if (pg_regex_locale->provider == COLLPROVIDER_ICU)
+		else if (locale->provider == COLLPROVIDER_ICU)
 		{
 			pg_regex_strategy = PG_REGEX_LOCALE_ICU;
 		}
@@ -278,6 +285,7 @@ pg_set_regex_collation(Oid collation)
 				pg_regex_strategy = PG_REGEX_LOCALE_1BYTE_L;
 		}
 
+		pg_regex_locale = locale;
 		pg_regex_collation = collation;
 	}
 }
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index 68069fcfd3..dc5178f56b 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -1636,6 +1636,7 @@ char *
 str_tolower(const char *buff, size_t nbytes, Oid collid)
 {
 	char	   *result;
+	pg_locale_t mylocale;
 
 	if (!buff)
 		return NULL;
@@ -1653,17 +1654,15 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
 				 errhint("Use the COLLATE clause to set the collation explicitly.")));
 	}
 
+	mylocale = pg_newlocale_from_collation(collid);
+
 	/* C/POSIX collations use this path regardless of database encoding */
-	if (lc_ctype_is_c(collid))
+	if (mylocale->ctype_is_c)
 	{
 		result = asc_tolower(buff, nbytes);
 	}
 	else
 	{
-		pg_locale_t mylocale;
-
-		mylocale = pg_newlocale_from_collation(collid);
-
 #ifdef USE_ICU
 		if (mylocale->provider == COLLPROVIDER_ICU)
 		{
@@ -1774,6 +1773,7 @@ char *
 str_toupper(const char *buff, size_t nbytes, Oid collid)
 {
 	char	   *result;
+	pg_locale_t mylocale;
 
 	if (!buff)
 		return NULL;
@@ -1791,17 +1791,15 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
 				 errhint("Use the COLLATE clause to set the collation explicitly.")));
 	}
 
+	mylocale = pg_newlocale_from_collation(collid);
+
 	/* C/POSIX collations use this path regardless of database encoding */
-	if (lc_ctype_is_c(collid))
+	if (mylocale->ctype_is_c)
 	{
 		result = asc_toupper(buff, nbytes);
 	}
 	else
 	{
-		pg_locale_t mylocale;
-
-		mylocale = pg_newlocale_from_collation(collid);
-
 #ifdef USE_ICU
 		if (mylocale->provider == COLLPROVIDER_ICU)
 		{
@@ -1954,6 +1952,7 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
 {
 	char	   *result;
 	int			wasalnum = false;
+	pg_locale_t mylocale;
 
 	if (!buff)
 		return NULL;
@@ -1971,17 +1970,15 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
 				 errhint("Use the COLLATE clause to set the collation explicitly.")));
 	}
 
+	mylocale = pg_newlocale_from_collation(collid);
+
 	/* C/POSIX collations use this path regardless of database encoding */
-	if (lc_ctype_is_c(collid))
+	if (mylocale->ctype_is_c)
 	{
 		result = asc_initcap(buff, nbytes);
 	}
 	else
 	{
-		pg_locale_t mylocale;
-
-		mylocale = pg_newlocale_from_collation(collid);
-
 #ifdef USE_ICU
 		if (mylocale->provider == COLLPROVIDER_ICU)
 		{
diff --git a/src/backend/utils/adt/like.c b/src/backend/utils/adt/like.c
index 131616fa6b..f87675d755 100644
--- a/src/backend/utils/adt/like.c
+++ b/src/backend/utils/adt/like.c
@@ -147,7 +147,7 @@ SB_lower_char(unsigned char c, pg_locale_t locale, bool locale_is_c)
 static inline int
 GenericMatchText(const char *s, int slen, const char *p, int plen, Oid collation)
 {
-	if (collation && !lc_ctype_is_c(collation))
+	if (collation)
 	{
 		pg_locale_t locale = pg_newlocale_from_collation(collation);
 
diff --git a/src/backend/utils/adt/like_support.c b/src/backend/utils/adt/like_support.c
index a579fa0157..fcfd2576b2 100644
--- a/src/backend/utils/adt/like_support.c
+++ b/src/backend/utils/adt/like_support.c
@@ -100,7 +100,7 @@ static Selectivity regex_selectivity(const char *patt, int pattlen,
 									 bool case_insensitive,
 									 int fixed_prefix_len);
 static int	pattern_char_isalpha(char c, bool is_multibyte,
-								 pg_locale_t locale, bool locale_is_c);
+								 pg_locale_t locale);
 static Const *make_greater_string(const Const *str_const, FmgrInfo *ltproc,
 								  Oid collation);
 static Datum string_to_datum(const char *str, Oid datatype);
@@ -1000,7 +1000,6 @@ like_fixed_prefix(Const *patt_const, bool case_insensitive, Oid collation,
 				match_pos;
 	bool		is_multibyte = (pg_database_encoding_max_length() > 1);
 	pg_locale_t locale = 0;
-	bool		locale_is_c = false;
 
 	/* the right-hand const is type text or bytea */
 	Assert(typeid == BYTEAOID || typeid == TEXTOID);
@@ -1024,11 +1023,7 @@ like_fixed_prefix(Const *patt_const, bool case_insensitive, Oid collation,
 					 errhint("Use the COLLATE clause to set the collation explicitly.")));
 		}
 
-		/* If case-insensitive, we need locale info */
-		if (lc_ctype_is_c(collation))
-			locale_is_c = true;
-		else
-			locale = pg_newlocale_from_collation(collation);
+		locale = pg_newlocale_from_collation(collation);
 	}
 
 	if (typeid != BYTEAOID)
@@ -1065,7 +1060,7 @@ like_fixed_prefix(Const *patt_const, bool case_insensitive, Oid collation,
 
 		/* Stop if case-varying character (it's sort of a wildcard) */
 		if (case_insensitive &&
-			pattern_char_isalpha(patt[pos], is_multibyte, locale, locale_is_c))
+			pattern_char_isalpha(patt[pos], is_multibyte, locale))
 			break;
 
 		match[match_pos++] = patt[pos];
@@ -1499,9 +1494,9 @@ regex_selectivity(const char *patt, int pattlen, bool case_insensitive,
  */
 static int
 pattern_char_isalpha(char c, bool is_multibyte,
-					 pg_locale_t locale, bool locale_is_c)
+					 pg_locale_t locale)
 {
-	if (locale_is_c)
+	if (locale->ctype_is_c)
 		return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
 	else if (is_multibyte && IS_HIGHBIT_SET(c))
 		return true;
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 7512ac055e..b1663e2b6e 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1258,32 +1258,6 @@ lookup_collation_cache(Oid collation)
 	return cache_entry;
 }
 
-/*
- * Detect whether collation's LC_CTYPE property is C
- */
-bool
-lc_ctype_is_c(Oid collation)
-{
-	/*
-	 * If we're asked about "collation 0", return false, so that the code will
-	 * go into the non-C path and report that the collation is bogus.
-	 */
-	if (!OidIsValid(collation))
-		return false;
-
-	/*
-	 * If we're asked about the built-in C/POSIX collations, we know that.
-	 */
-	if (collation == C_COLLATION_OID ||
-		collation == POSIX_COLLATION_OID)
-		return true;
-
-	/*
-	 * Otherwise, we have to consult pg_collation, but we cache that.
-	 */
-	return pg_newlocale_from_collation(collation)->ctype_is_c;
-}
-
 /* simple subroutine for reporting errors from newlocale() */
 static void
 report_newlocale_failure(const char *localename)
diff --git a/src/include/catalog/pg_collation.dat b/src/include/catalog/pg_collation.dat
index f126201276..af5c9aa582 100644
--- a/src/include/catalog/pg_collation.dat
+++ b/src/include/catalog/pg_collation.dat
@@ -19,8 +19,7 @@
   descr => 'standard C collation',
   collname => 'C', collprovider => 'c', collencoding => '-1',
   collcollate => 'C', collctype => 'C' },
-{ oid => '951', oid_symbol => 'POSIX_COLLATION_OID',
-  descr => 'standard POSIX collation',
+{ oid => '951', descr => 'standard POSIX collation',
   collname => 'POSIX', collprovider => 'c', collencoding => '-1',
   collcollate => 'POSIX', collctype => 'POSIX' },
 { oid => '962', descr => 'sorts by Unicode code point, C character semantics',
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index 8ec24437f4..ab1c37a44b 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -54,8 +54,6 @@ extern PGDLLIMPORT bool database_ctype_is_c;
 extern bool check_locale(int category, const char *locale, char **canonname);
 extern char *pg_perm_setlocale(int category, const char *locale);
 
-extern bool lc_ctype_is_c(Oid collation);
-
 /*
  * Return the POSIX lconv struct (contains number/money formatting
  * information) with locale information for all categories.
-- 
2.43.0

From 6a1c264ff11a1d745a789c06ea7d13a8c5e28363 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:47:08 +0200
Subject: [PATCH 3/6] Remove dubious check against default locale

---
 src/backend/utils/adt/varlena.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 81343bd0e1..1c02a67d72 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2548,7 +2548,6 @@ btvarstrequalimage(PG_FUNCTION_ARGS)
 	locale = pg_newlocale_from_collation(collid);
 
 	if (locale->collate_is_c ||
-		collid == DEFAULT_COLLATION_OID ||
 		pg_locale_deterministic(locale))
 		PG_RETURN_BOOL(true);
 	else
-- 
2.43.0

From 4e3dd6fcf2f33d709699216a7c7fa7f7dca54d44 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:49:46 +0200
Subject: [PATCH 4/6] Do not check both for collate_is_c and deterministic

Since deterimistic will always be true if collate_is_c we can just
check for deterimistic and make the code cleaner to read.
---
 src/backend/utils/adt/varchar.c | 4 ++--
 src/backend/utils/adt/varlena.c | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c
index d2d1c5709d..03e91c0333 100644
--- a/src/backend/utils/adt/varchar.c
+++ b/src/backend/utils/adt/varchar.c
@@ -757,7 +757,7 @@ bpchareq(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (mylocale->collate_is_c || pg_locale_deterministic(mylocale))
+	if (pg_locale_deterministic(mylocale))
 	{
 		/*
 		 * Since we only care about equality or not-equality, we can avoid all
@@ -798,7 +798,7 @@ bpcharne(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (mylocale->collate_is_c || pg_locale_deterministic(mylocale))
+	if (pg_locale_deterministic(mylocale))
 	{
 		/*
 		 * Since we only care about equality or not-equality, we can avoid all
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 1c02a67d72..2086c47e0f 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2547,8 +2547,7 @@ btvarstrequalimage(PG_FUNCTION_ARGS)
 
 	locale = pg_newlocale_from_collation(collid);
 
-	if (locale->collate_is_c ||
-		pg_locale_deterministic(locale))
+	if (pg_locale_deterministic(locale))
 		PG_RETURN_BOOL(true);
 	else
 		PG_RETURN_BOOL(false);
-- 
2.43.0

From 86d66751699a232beaf49e469ff6029a8b9caa21 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:52:53 +0200
Subject: [PATCH 5/6] Remove pg_collate_deterministic() and check field
 directly

There is no clear benefit from using this thin accessor function,
espeically since other fields like collate_is_c and ctype_is_c are
accessed directly.
---
 src/backend/access/hash/hashfunc.c |  4 ++--
 src/backend/regex/regc_pg_locale.c |  2 +-
 src/backend/utils/adt/like.c       |  4 ++--
 src/backend/utils/adt/pg_locale.c  |  7 -------
 src/backend/utils/adt/varchar.c    |  8 ++++----
 src/backend/utils/adt/varlena.c    | 14 +++++++-------
 src/include/utils/pg_locale.h      |  1 -
 7 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/src/backend/access/hash/hashfunc.c b/src/backend/access/hash/hashfunc.c
index 86f1adecb7..01bdf997e3 100644
--- a/src/backend/access/hash/hashfunc.c
+++ b/src/backend/access/hash/hashfunc.c
@@ -279,7 +279,7 @@ hashtext(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (pg_locale_deterministic(mylocale))
+	if (mylocale->deterministic)
 	{
 		result = hash_any((unsigned char *) VARDATA_ANY(key),
 						  VARSIZE_ANY_EXHDR(key));
@@ -334,7 +334,7 @@ hashtextextended(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (pg_locale_deterministic(mylocale))
+	if (mylocale->deterministic)
 	{
 		result = hash_any_extended((unsigned char *) VARDATA_ANY(key),
 								   VARSIZE_ANY_EXHDR(key),
diff --git a/src/backend/regex/regc_pg_locale.c b/src/backend/regex/regc_pg_locale.c
index 21f640a7e1..1683d706b9 100644
--- a/src/backend/regex/regc_pg_locale.c
+++ b/src/backend/regex/regc_pg_locale.c
@@ -254,7 +254,7 @@ pg_set_regex_collation(Oid collation)
 	{
 		pg_locale_t locale = pg_newlocale_from_collation(collation);
 
-		if (!pg_locale_deterministic(locale))
+		if (!locale->deterministic)
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("nondeterministic collations are not supported for regular expressions")));
diff --git a/src/backend/utils/adt/like.c b/src/backend/utils/adt/like.c
index f87675d755..286b737fd7 100644
--- a/src/backend/utils/adt/like.c
+++ b/src/backend/utils/adt/like.c
@@ -151,7 +151,7 @@ GenericMatchText(const char *s, int slen, const char *p, int plen, Oid collation
 	{
 		pg_locale_t locale = pg_newlocale_from_collation(collation);
 
-		if (!pg_locale_deterministic(locale))
+		if (!locale->deterministic)
 			ereport(ERROR,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("nondeterministic collations are not supported for LIKE")));
@@ -188,7 +188,7 @@ Generic_Text_IC_like(text *str, text *pat, Oid collation)
 
 	locale = pg_newlocale_from_collation(collation);
 
-	if (!pg_locale_deterministic(locale))
+	if (!locale->deterministic)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("nondeterministic collations are not supported for ILIKE")));
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index b1663e2b6e..8e952c7f16 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1407,13 +1407,6 @@ make_icu_collator(const char *iculocstr,
 #endif							/* not USE_ICU */
 }
 
-
-bool
-pg_locale_deterministic(pg_locale_t locale)
-{
-	return locale->deterministic;
-}
-
 /*
  * Initialize default_locale with database locale settings.
  */
diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c
index 03e91c0333..8f86aca297 100644
--- a/src/backend/utils/adt/varchar.c
+++ b/src/backend/utils/adt/varchar.c
@@ -757,7 +757,7 @@ bpchareq(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (pg_locale_deterministic(mylocale))
+	if (mylocale->deterministic)
 	{
 		/*
 		 * Since we only care about equality or not-equality, we can avoid all
@@ -798,7 +798,7 @@ bpcharne(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (pg_locale_deterministic(mylocale))
+	if (mylocale->deterministic)
 	{
 		/*
 		 * Since we only care about equality or not-equality, we can avoid all
@@ -1005,7 +1005,7 @@ hashbpchar(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (pg_locale_deterministic(mylocale))
+	if (mylocale->deterministic)
 	{
 		result = hash_any((unsigned char *) keydata, keylen);
 	}
@@ -1061,7 +1061,7 @@ hashbpcharextended(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (pg_locale_deterministic(mylocale))
+	if (mylocale->deterministic)
 	{
 		result = hash_any_extended((unsigned char *) keydata, keylen,
 								   PG_GETARG_INT64(1));
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 2086c47e0f..bc79d71d70 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -1223,7 +1223,7 @@ text_position_setup(text *t1, text *t2, Oid collid, TextPositionState *state)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (!pg_locale_deterministic(mylocale))
+	if (!mylocale->deterministic)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("nondeterministic collations are not supported for substring searches")));
@@ -1573,7 +1573,7 @@ varstr_cmp(const char *arg1, int len1, const char *arg2, int len2, Oid collid)
 		result = pg_strncoll(arg1, len1, arg2, len2, mylocale);
 
 		/* Break tie if necessary. */
-		if (result == 0 && pg_locale_deterministic(mylocale))
+		if (result == 0 && mylocale->deterministic)
 		{
 			result = memcmp(arg1, arg2, Min(len1, len2));
 			if ((result == 0) && (len1 != len2))
@@ -1624,7 +1624,7 @@ texteq(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (pg_locale_deterministic(mylocale))
+	if (mylocale->deterministic)
 	{
 		Datum		arg1 = PG_GETARG_DATUM(0);
 		Datum		arg2 = PG_GETARG_DATUM(1);
@@ -1679,7 +1679,7 @@ textne(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (pg_locale_deterministic(mylocale))
+	if (mylocale->deterministic)
 	{
 		Datum		arg1 = PG_GETARG_DATUM(0);
 		Datum		arg2 = PG_GETARG_DATUM(1);
@@ -1792,7 +1792,7 @@ text_starts_with(PG_FUNCTION_ARGS)
 
 	mylocale = pg_newlocale_from_collation(collid);
 
-	if (!pg_locale_deterministic(mylocale))
+	if (!mylocale->deterministic)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("nondeterministic collations are not supported for substring searches")));
@@ -2204,7 +2204,7 @@ varstrfastcmp_locale(char *a1p, int len1, char *a2p, int len2, SortSupport ssup)
 	result = pg_strcoll(sss->buf1, sss->buf2, sss->locale);
 
 	/* Break tie if necessary. */
-	if (result == 0 && pg_locale_deterministic(sss->locale))
+	if (result == 0 && sss->locale->deterministic)
 		result = strcmp(sss->buf1, sss->buf2);
 
 	/* Cache result, perhaps saving an expensive strcoll() call next time */
@@ -2547,7 +2547,7 @@ btvarstrequalimage(PG_FUNCTION_ARGS)
 
 	locale = pg_newlocale_from_collation(collid);
 
-	if (pg_locale_deterministic(locale))
+	if (locale->deterministic)
 		PG_RETURN_BOOL(true);
 	else
 		PG_RETURN_BOOL(false);
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index ab1c37a44b..faae868bfc 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -108,7 +108,6 @@ extern void make_icu_collator(const char *iculocstr,
 							  const char *icurules,
 							  struct pg_locale_struct *resultp);
 
-extern bool pg_locale_deterministic(pg_locale_t locale);
 extern void init_database_collation(void);
 extern pg_locale_t pg_newlocale_from_collation(Oid collid);
 
-- 
2.43.0

From 9b47f1a5190dd8dcd182c9f8b380242bbc510ff9 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:03:17 +0200
Subject: [PATCH 6/6] Slightly refactor varstr_sortsupport() to improve
 readability

---
 src/backend/utils/adt/varlena.c | 36 ++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index bc79d71d70..a6a150e90e 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -1911,25 +1911,25 @@ varstr_sortsupport(SortSupport ssup, Oid typid, Oid collid)
 		}
 		else
 			ssup->comparator = varlenafastcmp_locale;
-	}
 
-	/*
-	 * Unfortunately, it seems that abbreviation for non-C collations is
-	 * broken on many common platforms; see pg_strxfrm_enabled().
-	 *
-	 * Even apart from the risk of broken locales, it's possible that there
-	 * are platforms where the use of abbreviated keys should be disabled at
-	 * compile time.  Having only 4 byte datums could make worst-case
-	 * performance drastically more likely, for example.  Moreover, macOS's
-	 * strxfrm() implementation is known to not effectively concentrate a
-	 * significant amount of entropy from the original string in earlier
-	 * transformed blobs.  It's possible that other supported platforms are
-	 * similarly encumbered.  So, if we ever get past disabling this
-	 * categorically, we may still want or need to disable it for particular
-	 * platforms.
-	 */
-	if (!collate_c && !pg_strxfrm_enabled(locale))
-		abbreviate = false;
+		/*
+		 * Unfortunately, it seems that abbreviation for non-C collations is
+		 * broken on many common platforms; see pg_strxfrm_enabled().
+		 *
+		 * Even apart from the risk of broken locales, it's possible that there
+		 * are platforms where the use of abbreviated keys should be disabled at
+		 * compile time.  Having only 4 byte datums could make worst-case
+		 * performance drastically more likely, for example.  Moreover, macOS's
+		 * strxfrm() implementation is known to not effectively concentrate a
+		 * significant amount of entropy from the original string in earlier
+		 * transformed blobs.  It's possible that other supported platforms are
+		 * similarly encumbered.  So, if we ever get past disabling this
+		 * categorically, we may still want or need to disable it for particular
+		 * platforms.
+		 */
+		if (!pg_strxfrm_enabled(locale))
+			abbreviate = false;
+	}
 
 	/*
 	 * If we're using abbreviated keys, or if we're using a locale-aware
-- 
2.43.0

---

From: Jeff Davis <[email-masked]>
Date: Wed, 14 Aug 2024 15:55:03 -0700

> Removes lc_collate_is_c().

This could use some performance testing, as the commit message says,
otherwise it looks good.

> Removes lc_ctype_is_c() and POSIX_COLLATION_OID which is no longer 

This overlaps a bit with what Peter already proposed here:

https://www.postgresql.org/message-id/4f562d84-87f4-44dc-8946-01d6c437936f%40eisentraut.org

right?

> locale is deterministic? Since we know we have a valid locale that 

Looks good to me. The code was correct in the sense that the default
collation is always deterministic, but (a) Peter is working on non-
deterministic default collations; and (b) it was a redundant check.

+1.

> field makes it seem like there is more to it so I suggest that we

+1. When I added that, there was also a NULL check, but that was
removed so we might as well just read the field.

> Small refactor to make a hard to read function a bit easier to read.

Looks good to me.

Regards,
	Jeff Davis

---

From: Andreas Karlsson <[email-masked]>
Date: Wed, 28 Aug 2024 18:43:49 +0200

> https://www.postgresql.org/message-id/4f562d84-87f4-44dc-8946-01d6c437936f%40eisentraut.org

Maybe I am missing something but not as far as I can see. It is related 
but I do not see any overlap.

> deterministic default collations; and (b) it was a redundant check.

Suspected so.

I have attached a set of rebased patches with an added assert. Maybe I 
should start a new thread though.

Andreas

From 9715b6f429907a9f284284e697727bfc7b4082e7 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:52:53 +0200
Subject: [PATCH v2 5/5] Remove pg_collate_deterministic() and check field
 directly

 

 
index 749ae6b4b2..1b8008a03e 100644

 
 
-	if (!pg_locale_deterministic(locale))
+	if (!locale->deterministic)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("nondeterministic collations are not supported for ILIKE")));
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 3e37837b8d..6af78301b7 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1408,13 +1408,6 @@ make_icu_collator(const char *iculocstr,
 #endif							/* not USE_ICU */
 }
 
-
-bool

 

 

index 3a0f8ab6b5..fe8ac9a756 100644

@@ -1567,7 +1567,7 @@ varstr_cmp(const char *arg1, int len1, const char *arg2, int len2, Oid collid)
 
@@ -1618,7 +1618,7 @@ texteq(PG_FUNCTION_ARGS)

@@ -1673,7 +1673,7 @@ textne(PG_FUNCTION_ARGS)

@@ -1786,7 +1786,7 @@ text_starts_with(PG_FUNCTION_ARGS)

@@ -2198,7 +2198,7 @@ varstrfastcmp_locale(char *a1p, int len1, char *a2p, int len2, SortSupport ssup)

 	/* Cache result, perhaps saving an expensive strcoll() call next time */
@@ -2537,7 +2537,7 @@ btvarstrequalimage(PG_FUNCTION_ARGS)

 
 
-- 
2.43.0

From e4742d692fbf8657e6710cd9d54173f890155249 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:49:46 +0200
Subject: [PATCH v2 4/5] Do not check both for collate_is_c and deterministic

 

 
-	if (mylocale->collate_is_c || pg_locale_deterministic(mylocale))
+	if (pg_locale_deterministic(mylocale))
 	{
 		/*
 		 * Since we only care about equality or not-equality, we can avoid all
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 306f876e94..3a0f8ab6b5 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2537,8 +2537,7 @@ btvarstrequalimage(PG_FUNCTION_ARGS)

From b79c102dc45ccf55dd4d2383624ae434c6c590e8 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:47:08 +0200
Subject: [PATCH v2 3/5] Remove dubious check against default locale

---

diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 6b664dfdd8..306f876e94 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2538,7 +2538,6 @@ btvarstrequalimage(PG_FUNCTION_ARGS)
 	locale = pg_newlocale_from_collation(collid);

From b8df8e4dd7fcaa20c9beb7710f91c96142d87a71 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:37:48 +0200
Subject: [PATCH v2 2/5] Remove lc_ctype_is_c()

 src/backend/regex/regc_pg_locale.c   | 21 +++++++++++++++------
 7 files changed, 34 insertions(+), 62 deletions(-)

diff --git a/src/backend/regex/regc_pg_locale.c b/src/backend/regex/regc_pg_locale.c
index 947d73f3e0..749ae6b4b2 100644
--- a/src/backend/regex/regc_pg_locale.c
+++ b/src/backend/regex/regc_pg_locale.c
@@ -243,41 +243,50 @@ pg_set_regex_collation(Oid collation)
 				 errhint("Use the COLLATE clause to set the collation explicitly.")));
 	}

 
+			Assert(pg_regex_locale->provider == COLLPROVIDER_LIBC);
 				pg_regex_strategy = PG_REGEX_LOCALE_WIDE_L;

 	if (!buff)
 		return NULL;

 	if (!buff)
 		return NULL;

 	if (!buff)
 		return NULL;

 

 	if (typeid != BYTEAOID)

index 4438b3817a..3e37837b8d 100644
@@ -1259,32 +1259,6 @@ lookup_collation_cache(Oid collation)
 	return cache_entry;
 }
 
-/*

From 849ab262a27017f5bbe1472b8f9888fbcab068f2 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email-masked]>
Date: Wed, 14 Aug 2024 00:37:23 +0200
Subject: [PATCH v2 1/5] Remove lc_collate_is_c()

 
-	/*

 
 	/*
index 643cca05d3..4438b3817a 100644
@@ -1259,33 +1259,6 @@ lookup_collation_cache(Oid collation)
 	return cache_entry;
 }
 
-
-/*
 
 	switch (typid)
 	{

 
 	len1 = bcTruelen(arg1);
 	len2 = bcTruelen(arg2);

 
 
 	len1 = bcTruelen(arg1);
 	len2 = bcTruelen(arg2);

-	if (locale_is_c || pg_locale_deterministic(mylocale))
+	if (mylocale->collate_is_c || pg_locale_deterministic(mylocale))
 	{
 		/*
 		 * Since we only care about equality or not-equality, we can avoid all
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 7c6391a276..6b664dfdd8 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -1538,10 +1538,13 @@ int
 varstr_cmp(const char *arg1, int len1, const char *arg2, int len2, Oid collid)
 {
 	int			result;
+	pg_locale_t mylocale;

@@ -1549,10 +1552,6 @@ varstr_cmp(const char *arg1, int len1, const char *arg2, int len2, Oid collid)
@@ -1859,10 +1858,12 @@ varstr_sortsupport(SortSupport ssup, Oid typid, Oid collid)

@@ -1876,7 +1877,7 @@ varstr_sortsupport(SortSupport ssup, Oid typid, Oid collid)
@@ -1893,13 +1894,6 @@ varstr_sortsupport(SortSupport ssup, Oid typid, Oid collid)
@@ -1950,7 +1944,8 @@ varstr_sortsupport(SortSupport ssup, Oid typid, Oid collid)
 		sss->last_len2 = -1;
 		/* Initialize */
 		sss->last_returned = 0;
-		sss->locale = locale;
+		if (!collate_c)
+			sss->locale = locale;
 
 		/*
 		 * To avoid somehow confusing a strxfrm() blob and an original string,
@@ -2536,12 +2531,15 @@ btvarstrequalimage(PG_FUNCTION_ARGS)
 {
 	/* Oid		opcintype = PG_GETARG_OID(0); */
 	Oid			collid = PG_GET_COLLATION();
+	pg_locale_t locale;

 
 
 /*
-- 
2.43.0