Call pg_newlocale_from_collation() also with default collation

Previously, callers of pg_newlocale_from_collation() did not call it
if the collation was DEFAULT_COLLATION_OID and instead proceeded with
a pg_locale_t of 0.  Instead, now we call it anyway and have it return
0 if the default collation was passed.  It already did this, so we
just have to adjust the callers.  This simplifies all the call sites
and also makes future enhancements easier.

After discussion and testing, the previous comment in pg_locale.c
about avoiding this for performance reasons may have been mistaken
since it was testing a very different patch version way back when.

Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://www.postgresql.org/message-id/ed3baa81-7fac-7788-cc12-41e3f7917e34@enterprisedb.com

src/backend/access/hash/hashfunc.c

@@ -278,7 +278,7 @@ hashtext(PG_FUNCTION_ARGS)
 				 errmsg("could not determine which collation to use for string hashing"),
 				 errhint("Use the COLLATE clause to set the collation explicitly.")));
 
-	if (!lc_collate_is_c(collid) && collid != DEFAULT_COLLATION_OID)
+	if (!lc_collate_is_c(collid))
 		mylocale = pg_newlocale_from_collation(collid);
 
 	if (!mylocale || mylocale->deterministic)
@@ -334,7 +334,7 @@ hashtextextended(PG_FUNCTION_ARGS)
 				 errmsg("could not determine which collation to use for string hashing"),
 				 errhint("Use the COLLATE clause to set the collation explicitly.")));
 
-	if (!lc_collate_is_c(collid) && collid != DEFAULT_COLLATION_OID)
+	if (!lc_collate_is_c(collid))
 		mylocale = pg_newlocale_from_collation(collid);
 
 	if (!mylocale || mylocale->deterministic)

src/backend/regex/regc_pg_locale.c

@@ -231,6 +231,18 @@ static const unsigned char pg_char_properties[128] = {
 void
 pg_set_regex_collation(Oid collation)
 {
+	if (!OidIsValid(collation))
+	{
+		/*
+		 * This typically means that the parser could not resolve a
+		 * conflict of implicit collations, so report it that way.
+		 */
+		ereport(ERROR,
+				(errcode(ERRCODE_INDETERMINATE_COLLATION),
+				 errmsg("could not determine which collation to use for regular expression"),
+				 errhint("Use the COLLATE clause to set the collation explicitly.")));
+	}
+
 	if (lc_ctype_is_c(collation))
 	{
 		/* C/POSIX collations use this path regardless of database encoding */
@@ -239,10 +251,6 @@ pg_set_regex_collation(Oid collation)
 		pg_regex_collation = C_COLLATION_OID;
 	}
 	else
-	{
-		if (collation == DEFAULT_COLLATION_OID)
-			pg_regex_locale = 0;
-		else if (OidIsValid(collation))
 	{
 		/*
 		 * NB: pg_newlocale_from_collation will fail if not HAVE_LOCALE_T;
@@ -250,18 +258,6 @@ pg_set_regex_collation(Oid collation)
 		 * have to be considered below.
 		 */
 		pg_regex_locale = pg_newlocale_from_collation(collation);
-		}
-		else
-		{
-			/*
-			 * This typically means that the parser could not resolve a
-			 * conflict of implicit collations, so report it that way.
-			 */
-			ereport(ERROR,
-					(errcode(ERRCODE_INDETERMINATE_COLLATION),
-					 errmsg("could not determine which collation to use for regular expression"),
-					 errhint("Use the COLLATE clause to set the collation explicitly.")));
-		}
 
 		if (pg_regex_locale && !pg_regex_locale->deterministic)
 			ereport(ERROR,

src/backend/utils/adt/formatting.c

@@ -1641,17 +1641,6 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
 	if (!buff)
 		return NULL;
 
-	/* C/POSIX collations use this path regardless of database encoding */
-	if (lc_ctype_is_c(collid))
-	{
-		result = asc_tolower(buff, nbytes);
-	}
-	else
-	{
-		pg_locale_t mylocale = 0;
-
-		if (collid != DEFAULT_COLLATION_OID)
-		{
 	if (!OidIsValid(collid))
 	{
 		/*
@@ -1664,8 +1653,17 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
 						"lower()"),
 				 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))
+	{
+		result = asc_tolower(buff, nbytes);
 	}
+	else
+	{
+		pg_locale_t mylocale;
+
+		mylocale = pg_newlocale_from_collation(collid);
 
 #ifdef USE_ICU
 		if (mylocale && mylocale->provider == COLLPROVIDER_ICU)
@@ -1765,17 +1763,6 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
 	if (!buff)
 		return NULL;
 
-	/* C/POSIX collations use this path regardless of database encoding */
-	if (lc_ctype_is_c(collid))
-	{
-		result = asc_toupper(buff, nbytes);
-	}
-	else
-	{
-		pg_locale_t mylocale = 0;
-
-		if (collid != DEFAULT_COLLATION_OID)
-		{
 	if (!OidIsValid(collid))
 	{
 		/*
@@ -1788,8 +1775,17 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
 						"upper()"),
 				 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))
+	{
+		result = asc_toupper(buff, nbytes);
 	}
+	else
+	{
+		pg_locale_t mylocale;
+
+		mylocale = pg_newlocale_from_collation(collid);
 
 #ifdef USE_ICU
 		if (mylocale && mylocale->provider == COLLPROVIDER_ICU)
@@ -1890,17 +1886,6 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
 	if (!buff)
 		return NULL;
 
-	/* C/POSIX collations use this path regardless of database encoding */
-	if (lc_ctype_is_c(collid))
-	{
-		result = asc_initcap(buff, nbytes);
-	}
-	else
-	{
-		pg_locale_t mylocale = 0;
-
-		if (collid != DEFAULT_COLLATION_OID)
-		{
 	if (!OidIsValid(collid))
 	{
 		/*
@@ -1913,8 +1898,17 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
 						"initcap()"),
 				 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))
+	{
+		result = asc_initcap(buff, nbytes);
 	}
+	else
+	{
+		pg_locale_t mylocale;
+
+		mylocale = pg_newlocale_from_collation(collid);
 
 #ifdef USE_ICU
 		if (mylocale && mylocale->provider == COLLPROVIDER_ICU)

src/backend/utils/adt/like.c

@@ -150,7 +150,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) && collation != DEFAULT_COLLATION_OID)
+	if (collation && !lc_ctype_is_c(collation))
 	{
 		pg_locale_t locale = pg_newlocale_from_collation(collation);
 
@@ -178,10 +178,6 @@ Generic_Text_IC_like(text *str, text *pat, Oid collation)
 	pg_locale_t locale = 0;
 	bool		locale_is_c = false;
 
-	if (lc_ctype_is_c(collation))
-		locale_is_c = true;
-	else if (collation != DEFAULT_COLLATION_OID)
-	{
 	if (!OidIsValid(collation))
 	{
 		/*
@@ -193,13 +189,16 @@ Generic_Text_IC_like(text *str, text *pat, Oid collation)
 				 errmsg("could not determine which collation to use for ILIKE"),
 				 errhint("Use the COLLATE clause to set the collation explicitly.")));
 	}
+
+	if (lc_ctype_is_c(collation))
+		locale_is_c = true;
+	else
 		locale = pg_newlocale_from_collation(collation);
 
 	if (locale && !locale->deterministic)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("nondeterministic collations are not supported for ILIKE")));
-	}
 
 	/*
 	 * For efficiency reasons, in the single byte case we don't call lower()

src/backend/utils/adt/like_support.c

@@ -1012,11 +1012,6 @@ like_fixed_prefix(Const *patt_const, bool case_insensitive, Oid collation,
 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 					 errmsg("case insensitive matching not supported on type bytea")));
 
-		/* If case-insensitive, we need locale info */
-		if (lc_ctype_is_c(collation))
-			locale_is_c = true;
-		else if (collation != DEFAULT_COLLATION_OID)
-		{
 		if (!OidIsValid(collation))
 		{
 			/*
@@ -1028,9 +1023,13 @@ like_fixed_prefix(Const *patt_const, bool case_insensitive, Oid collation,
 					 errmsg("could not determine which collation to use for ILIKE"),
 					 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);
 	}
-	}
 
 	if (typeid != BYTEAOID)
 	{

src/backend/utils/adt/pg_locale.c

@@ -1454,8 +1454,6 @@ report_newlocale_failure(const char *localename)
  *
  * As a special optimization, the default/database collation returns 0.
  * Callers should then revert to the non-locale_t-enabled code path.
- * In fact, they shouldn't call this function at all when they are dealing
- * with the default locale.  That can save quite a bit in hotspots.
  * Also, callers should avoid calling this before going down a C/POSIX
  * fastpath, because such a fastpath should work even on platforms without
  * locale_t support in the C library.
@@ -1472,7 +1470,6 @@ pg_newlocale_from_collation(Oid collid)
 	/* Callers must pass a valid OID */
 	Assert(OidIsValid(collid));
 
-	/* Return 0 for "default" collation, just in case caller forgets */
 	if (collid == DEFAULT_COLLATION_OID)
 		return (pg_locale_t) 0;
 

src/backend/utils/adt/varchar.c

@@ -743,15 +743,20 @@ bpchareq(PG_FUNCTION_ARGS)
 				len2;
 	bool		result;
 	Oid			collid = PG_GET_COLLATION();
+	bool		locale_is_c = false;
+	pg_locale_t	mylocale = 0;
 
 	check_collation_set(collid);
 
 	len1 = bcTruelen(arg1);
 	len2 = bcTruelen(arg2);
 
-	if (lc_collate_is_c(collid) ||
+	if (lc_collate_is_c(collid))
-		collid == DEFAULT_COLLATION_OID ||
+		locale_is_c = true;
-		pg_newlocale_from_collation(collid)->deterministic)
+	else
+		mylocale = pg_newlocale_from_collation(collid);
+
+	if (locale_is_c || !mylocale || mylocale->deterministic)
 	{
 		/*
 		 * Since we only care about equality or not-equality, we can avoid all
@@ -783,15 +788,20 @@ bpcharne(PG_FUNCTION_ARGS)
 				len2;
 	bool		result;
 	Oid			collid = PG_GET_COLLATION();
+	bool		locale_is_c = false;
+	pg_locale_t	mylocale = 0;
 
 	check_collation_set(collid);
 
 	len1 = bcTruelen(arg1);
 	len2 = bcTruelen(arg2);
 
-	if (lc_collate_is_c(collid) ||
+	if (lc_collate_is_c(collid))
-		collid == DEFAULT_COLLATION_OID ||
+		locale_is_c = true;
-		pg_newlocale_from_collation(collid)->deterministic)
+	else
+		mylocale = pg_newlocale_from_collation(collid);
+
+	if (locale_is_c || !mylocale || mylocale->deterministic)
 	{
 		/*
 		 * Since we only care about equality or not-equality, we can avoid all
@@ -996,7 +1006,7 @@ hashbpchar(PG_FUNCTION_ARGS)
 	keydata = VARDATA_ANY(key);
 	keylen = bcTruelen(key);
 
-	if (!lc_collate_is_c(collid) && collid != DEFAULT_COLLATION_OID)
+	if (!lc_collate_is_c(collid))
 		mylocale = pg_newlocale_from_collation(collid);
 
 	if (!mylocale || mylocale->deterministic)
@@ -1056,7 +1066,7 @@ hashbpcharextended(PG_FUNCTION_ARGS)
 	keydata = VARDATA_ANY(key);
 	keylen = bcTruelen(key);
 
-	if (!lc_collate_is_c(collid) && collid != DEFAULT_COLLATION_OID)
+	if (!lc_collate_is_c(collid))
 		mylocale = pg_newlocale_from_collation(collid);
 
 	if (!mylocale || mylocale->deterministic)

src/backend/utils/adt/varlena.c

@@ -1200,7 +1200,7 @@ text_position_setup(text *t1, text *t2, Oid collid, TextPositionState *state)
 
 	check_collation_set(collid);
 
-	if (!lc_collate_is_c(collid) && collid != DEFAULT_COLLATION_OID)
+	if (!lc_collate_is_c(collid))
 		mylocale = pg_newlocale_from_collation(collid);
 
 	if (mylocale && !mylocale->deterministic)
@@ -1556,9 +1556,8 @@ varstr_cmp(const char *arg1, int len1, const char *arg2, int len2, Oid collid)
 		char		a2buf[TEXTBUFLEN];
 		char	   *a1p,
 				   *a2p;
-		pg_locale_t mylocale = 0;
+		pg_locale_t mylocale;
 
-		if (collid != DEFAULT_COLLATION_OID)
 		mylocale = pg_newlocale_from_collation(collid);
 
 		/*
@@ -1776,13 +1775,18 @@ Datum
 texteq(PG_FUNCTION_ARGS)
 {
 	Oid			collid = PG_GET_COLLATION();
+	bool		locale_is_c = false;
+	pg_locale_t	mylocale = 0;
 	bool		result;
 
 	check_collation_set(collid);
 
-	if (lc_collate_is_c(collid) ||
+	if (lc_collate_is_c(collid))
-		collid == DEFAULT_COLLATION_OID ||
+		locale_is_c = true;
-		pg_newlocale_from_collation(collid)->deterministic)
+	else
+		mylocale = pg_newlocale_from_collation(collid);
+
+	if (locale_is_c || !mylocale || mylocale->deterministic)
 	{
 		Datum		arg1 = PG_GETARG_DATUM(0);
 		Datum		arg2 = PG_GETARG_DATUM(1);
@@ -1830,13 +1834,18 @@ Datum
 textne(PG_FUNCTION_ARGS)
 {
 	Oid			collid = PG_GET_COLLATION();
+	bool		locale_is_c = false;
+	pg_locale_t	mylocale = 0;
 	bool		result;
 
 	check_collation_set(collid);
 
-	if (lc_collate_is_c(collid) ||
+	if (lc_collate_is_c(collid))
-		collid == DEFAULT_COLLATION_OID ||
+		locale_is_c = true;
-		pg_newlocale_from_collation(collid)->deterministic)
+	else
+		mylocale = pg_newlocale_from_collation(collid);
+
+	if (locale_is_c || !mylocale || mylocale->deterministic)
 	{
 		Datum		arg1 = PG_GETARG_DATUM(0);
 		Datum		arg2 = PG_GETARG_DATUM(1);
@@ -1947,7 +1956,7 @@ text_starts_with(PG_FUNCTION_ARGS)
 
 	check_collation_set(collid);
 
-	if (!lc_collate_is_c(collid) && collid != DEFAULT_COLLATION_OID)
+	if (!lc_collate_is_c(collid))
 		mylocale = pg_newlocale_from_collation(collid);
 
 	if (mylocale && !mylocale->deterministic)
@@ -2061,7 +2070,6 @@ varstr_sortsupport(SortSupport ssup, Oid typid, Oid collid)
 		 * we'll figure out the collation based on the locale id and cache the
 		 * result.
 		 */
-		if (collid != DEFAULT_COLLATION_OID)
 		locale = pg_newlocale_from_collation(collid);
 
 		/*