public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v49 2/7] Add conditional lock feature to dshash
6+ messages / 4 participants
[nested] [flat]

* [PATCH v49 2/7] Add conditional lock feature to dshash
@ 2020-03-13 07:58  Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Kyotaro Horiguchi @ 2020-03-13 07:58 UTC (permalink / raw)

Dshash currently waits for lock unconditionally. It is inconvenient
when we want to avoid being blocked by other processes. This commit
adds alternative functions of dshash_find and dshash_find_or_insert
that allows immediate return on lock failure.
---
 src/backend/lib/dshash.c | 98 +++++++++++++++++++++-------------------
 src/include/lib/dshash.h |  3 ++
 2 files changed, 55 insertions(+), 46 deletions(-)

diff --git a/src/backend/lib/dshash.c b/src/backend/lib/dshash.c
index 520bfa0979..853d78b528 100644
--- a/src/backend/lib/dshash.c
+++ b/src/backend/lib/dshash.c
@@ -383,6 +383,10 @@ dshash_get_hash_table_handle(dshash_table *hash_table)
  * the caller must take care to ensure that the entry is not left corrupted.
  * The lock mode is either shared or exclusive depending on 'exclusive'.
  *
+ * If found is not NULL, *found is set to true if the key is found in the hash
+ * table. If the key is not found, *found is set to false and a pointer to a
+ * newly created entry is returned.
+ *
  * The caller must not lock a lock already.
  *
  * Note that the lock held is in fact an LWLock, so interrupts will be held on
@@ -392,36 +396,7 @@ dshash_get_hash_table_handle(dshash_table *hash_table)
 void *
 dshash_find(dshash_table *hash_table, const void *key, bool exclusive)
 {
-	dshash_hash hash;
-	size_t		partition;
-	dshash_table_item *item;
-
-	hash = hash_key(hash_table, key);
-	partition = PARTITION_FOR_HASH(hash);
-
-	Assert(hash_table->control->magic == DSHASH_MAGIC);
-	Assert(!hash_table->find_locked);
-
-	LWLockAcquire(PARTITION_LOCK(hash_table, partition),
-				  exclusive ? LW_EXCLUSIVE : LW_SHARED);
-	ensure_valid_bucket_pointers(hash_table);
-
-	/* Search the active bucket. */
-	item = find_in_bucket(hash_table, key, BUCKET_FOR_HASH(hash_table, hash));
-
-	if (!item)
-	{
-		/* Not found. */
-		LWLockRelease(PARTITION_LOCK(hash_table, partition));
-		return NULL;
-	}
-	else
-	{
-		/* The caller will free the lock by calling dshash_release_lock. */
-		hash_table->find_locked = true;
-		hash_table->find_exclusively_locked = exclusive;
-		return ENTRY_FROM_ITEM(item);
-	}
+	return dshash_find_extended(hash_table, key, exclusive, false, false, NULL);
 }
 
 /*
@@ -439,31 +414,60 @@ dshash_find_or_insert(dshash_table *hash_table,
 					  const void *key,
 					  bool *found)
 {
-	dshash_hash hash;
-	size_t		partition_index;
-	dshash_partition *partition;
+	return dshash_find_extended(hash_table, key, true, false, true, found);
+}
+
+
+/*
+ * Find the key in the hash table.
+ *
+ * "exclusive" is the lock mode in which the partition for the returned item
+ * is locked.  If "nowait" is true, the function immediately returns if
+ * required lock was not acquired.  "insert" indicates insert mode. In this
+ * mode new entry is inserted and set *found to false. *found is set to true if
+ * found. "found" must be non-null in this mode.
+ */
+void *
+dshash_find_extended(dshash_table *hash_table, const void *key,
+					 bool exclusive, bool nowait, bool insert, bool *found)
+{
+	dshash_hash hash = hash_key(hash_table, key);
+	size_t		partidx = PARTITION_FOR_HASH(hash);
+	dshash_partition *partition = &hash_table->control->partitions[partidx];
+	LWLockMode  lockmode = exclusive ? LW_EXCLUSIVE : LW_SHARED;
 	dshash_table_item *item;
 
-	hash = hash_key(hash_table, key);
-	partition_index = PARTITION_FOR_HASH(hash);
-	partition = &hash_table->control->partitions[partition_index];
-
-	Assert(hash_table->control->magic == DSHASH_MAGIC);
-	Assert(!hash_table->find_locked);
+	/* must be exclusive when insert allowed */
+	Assert(!insert || (exclusive && found != NULL));
 
 restart:
-	LWLockAcquire(PARTITION_LOCK(hash_table, partition_index),
-				  LW_EXCLUSIVE);
+	if (!nowait)
+		LWLockAcquire(PARTITION_LOCK(hash_table, partidx), lockmode);
+	else if (!LWLockConditionalAcquire(PARTITION_LOCK(hash_table, partidx),
+									   lockmode))
+		return NULL;
+
 	ensure_valid_bucket_pointers(hash_table);
 
 	/* Search the active bucket. */
 	item = find_in_bucket(hash_table, key, BUCKET_FOR_HASH(hash_table, hash));
 
 	if (item)
-		*found = true;
+	{
+		if (found)
+			*found = true;
+	}
 	else
 	{
-		*found = false;
+		if (found)
+			*found = false;
+
+		if (!insert)
+		{
+			/* The caller didn't told to add a new entry. */
+			LWLockRelease(PARTITION_LOCK(hash_table, partidx));
+			return NULL;
+		}
 
 		/* Check if we are getting too full. */
 		if (partition->count > MAX_COUNT_PER_PARTITION(hash_table))
@@ -479,7 +483,8 @@ restart:
 			 * Give up our existing lock first, because resizing needs to
 			 * reacquire all the locks in the right order to avoid deadlocks.
 			 */
-			LWLockRelease(PARTITION_LOCK(hash_table, partition_index));
+			LWLockRelease(PARTITION_LOCK(hash_table, partidx));
+
 			resize(hash_table, hash_table->size_log2 + 1);
 
 			goto restart;
@@ -493,12 +498,13 @@ restart:
 		++partition->count;
 	}
 
-	/* The caller must release the lock with dshash_release_lock. */
+	/* The caller will free the lock by calling dshash_release_lock. */
 	hash_table->find_locked = true;
-	hash_table->find_exclusively_locked = true;
+	hash_table->find_exclusively_locked = exclusive;
 	return ENTRY_FROM_ITEM(item);
 }
 
+
 /*
  * Remove an entry by key.  Returns true if the key was found and the
  * corresponding entry was removed.
diff --git a/src/include/lib/dshash.h b/src/include/lib/dshash.h
index a6ea377173..5b8114d041 100644
--- a/src/include/lib/dshash.h
+++ b/src/include/lib/dshash.h
@@ -91,6 +91,9 @@ extern void *dshash_find(dshash_table *hash_table,
 						 const void *key, bool exclusive);
 extern void *dshash_find_or_insert(dshash_table *hash_table,
 								   const void *key, bool *found);
+extern void *dshash_find_extended(dshash_table *hash_table, const void *key,
+								  bool exclusive, bool nowait, bool insert,
+								  bool *found);
 extern bool dshash_delete_key(dshash_table *hash_table, const void *key);
 extern void dshash_delete_entry(dshash_table *hash_table, void *entry);
 extern void dshash_release_lock(dshash_table *hash_table, void *entry);
-- 
2.27.0


----Next_Part(Tue_Mar__9_16_53_11_2021_575)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="v49-0003-Make-archiver-process-an-auxiliary-process.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: check_strxfrm_bug()
@ 2023-06-12 08:48  Heikki Linnakangas <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Heikki Linnakangas @ 2023-06-12 08:48 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; pgsql-hackers

On 12/06/2023 01:15, Thomas Munro wrote:
> There are still at least a couple
> of functions that lack XXX_l variants in the standard: mbstowcs() and
> wcstombs() (though we use the non-standard _l variants if we find them
> in <xlocale.h>), but that's OK because we use uselocale() and not
> setlocale(), because uselocale() is required to be thread-local.

Right, mbstowcs() and wcstombs() are already thread-safe, that's why 
there are no _l variants of them.

> The use of setlocale() to set up the per-backend/per-database
> default locale would have to be replaced with uselocale().
This recent bug report is also related to that: 
https://www.postgresql.org/message-id/17946-3e84cb577e9551c3%40postgresql.org. 
In a nutshell, libperl calls uselocale(), which overrides the setting we 
try set with setlocale().

-- 
Heikki Linnakangas
Neon (https://neon.tech)







^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: check_strxfrm_bug()
@ 2023-06-27 23:03  Thomas Munro <[email protected]>
  parent: Heikki Linnakangas <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Thomas Munro @ 2023-06-27 23:03 UTC (permalink / raw)
  To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers

The GCC build farm has just received some SPARC hardware new enough to
run modern Solaris (hostname gcc106), so if wrasse were moved over
there we could finally assume all systems have POSIX 2008 (AKA
SUSv4)'s locale_t.

It's slightly annoying that Windows has locale_t but doesn't have
uselocale().  It does have thread-local locales via another API,
though.  I wonder how hard it would be to get to a point where all
systems have uselocale() too, by supplying a replacement.  I noticed
that some other projects eg older versions of LLVM libcxx do that.  I
see from one of their discussions[1] that it worked, except that
thread-local locales are only available with one of the MinGW C
runtimes and not another.  We'd have to get to the bottom of that.

[1] https://reviews.llvm.org/D40181






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: check_strxfrm_bug()
@ 2023-06-28 01:02  Thomas Munro <[email protected]>
  parent: Thomas Munro <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Thomas Munro @ 2023-06-28 01:02 UTC (permalink / raw)
  To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers

On Wed, Jun 28, 2023 at 11:03 AM Thomas Munro <[email protected]> wrote:
> The GCC build farm has just received some SPARC hardware new enough to
> run modern Solaris (hostname gcc106), so if wrasse were moved over
> there we could finally assume all systems have POSIX 2008 (AKA
> SUSv4)'s locale_t.

That would look something like this.


Attachments:

  [text/x-patch] 0001-All-supported-computers-have-locale_t.patch (21.8K, ../../CA+hUKGKVzrKmJ6JLsficC3GnGo0j9hTfFaJLUXYzQs5hY85U6Q@mail.gmail.com/2-0001-All-supported-computers-have-locale_t.patch)
  download | inline diff:
From 87268870aa9b6f22eb0bf831614bd38c529b0c8c Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Wed, 28 Jun 2023 10:23:32 +1200
Subject: [PATCH] All supported computers have locale_t.

locale_t is defined by POSIX:1-2008 and SUSv4, and available on all
targeted systems.

Definitions for HAVE_WCSTOMBS_L and HAVE_MBSTOWCS_L had to be moved into
win32_port.h, because this change revealed that MinGW builds on Windows
were failing to detect these functions, but without them we couldn't
build due to the assumption that uselocale() exists.

Discussion: https://postgr.es/m/CA%2BhUKGLg7_T2GKwZFAkEf0V7vbnur-NfCjZPKZb%3DZfAXSV1ORw%40mail.gmail.com

diff --git a/config/c-library.m4 b/config/c-library.m4
index c1dd804679..aa8223d2ef 100644
--- a/config/c-library.m4
+++ b/config/c-library.m4
@@ -86,9 +86,9 @@ AC_DEFUN([PGAC_STRUCT_SOCKADDR_SA_LEN],
 # PGAC_TYPE_LOCALE_T
 # ------------------
 # Check for the locale_t type and find the right header file.  macOS
-# needs xlocale.h; standard is locale.h, but glibc also has an
-# xlocale.h file that we should not use.
-#
+# needs xlocale.h; standard is locale.h, but glibc <= 2.25 also had an
+# xlocale.h file that we should not use, so we check the standard
+# header first.
 AC_DEFUN([PGAC_TYPE_LOCALE_T],
 [AC_CACHE_CHECK([for locale_t], pgac_cv_type_locale_t,
 [AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
@@ -102,10 +102,6 @@ locale_t x;],
 [])],
 [pgac_cv_type_locale_t='yes (in xlocale.h)'],
 [pgac_cv_type_locale_t=no])])])
-if test "$pgac_cv_type_locale_t" != no; then
-  AC_DEFINE(HAVE_LOCALE_T, 1,
-            [Define to 1 if the system has the type `locale_t'.])
-fi
 if test "$pgac_cv_type_locale_t" = 'yes (in xlocale.h)'; then
   AC_DEFINE(LOCALE_T_IN_XLOCALE, 1,
             [Define to 1 if `locale_t' requires <xlocale.h>.])
diff --git a/configure b/configure
index a1c3cb0036..b8bc03e2c7 100755
--- a/configure
+++ b/configure
@@ -15122,11 +15122,6 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
 fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_type_locale_t" >&5
 $as_echo "$pgac_cv_type_locale_t" >&6; }
-if test "$pgac_cv_type_locale_t" != no; then
-
-$as_echo "#define HAVE_LOCALE_T 1" >>confdefs.h
-
-fi
 if test "$pgac_cv_type_locale_t" = 'yes (in xlocale.h)'; then
 
 $as_echo "#define LOCALE_T_IN_XLOCALE 1" >>confdefs.h
diff --git a/meson.build b/meson.build
index 7bc7310c44..7cc4cbc034 100644
--- a/meson.build
+++ b/meson.build
@@ -2283,17 +2283,12 @@ else
   cdata.set('STRERROR_R_INT', false)
 endif
 
-# Check for the locale_t type and find the right header file.  macOS
-# needs xlocale.h; standard is locale.h, but glibc also has an
+# Find the right header file for the locale_t type.  macOS
+# needs xlocale.h; standard is locale.h, but older glibc also had an
 # xlocale.h file that we should not use.  MSVC has a replacement
 # defined in src/include/port/win32_port.h.
-if cc.has_type('locale_t', prefix: '#include <locale.h>')
-  cdata.set('HAVE_LOCALE_T', 1)
-elif cc.has_type('locale_t', prefix: '#include <xlocale.h>')
-  cdata.set('HAVE_LOCALE_T', 1)
+if not cc.has_type('locale_t', prefix: '#include <locale.h>') and cc.has_type('locale_t', prefix: '#include <xlocale.h>')
   cdata.set('LOCALE_T_IN_XLOCALE', 1)
-elif cc.get_id() == 'msvc'
-  cdata.set('HAVE_LOCALE_T', 1)
 endif
 
 # Check if the C compiler understands typeof or a variant.  Define
@@ -2489,13 +2484,6 @@ if cc.has_function('syslog', args: test_c_args) and \
 endif
 
 
-# MSVC has replacements defined in src/include/port/win32_port.h.
-if cc.get_id() == 'msvc'
-  cdata.set('HAVE_WCSTOMBS_L', 1)
-  cdata.set('HAVE_MBSTOWCS_L', 1)
-endif
-
-
 # if prerequisites for unnamed posix semas aren't fulfilled, fall back to sysv
 # semaphores
 if sema_kind == 'unnamed_posix' and \
diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c
index efb8b4d289..cc239b4d14 100644
--- a/src/backend/commands/collationcmds.c
+++ b/src/backend/commands/collationcmds.c
@@ -534,7 +534,7 @@ pg_collation_actual_version(PG_FUNCTION_ARGS)
 
 
 /* will we use "locale -a" in pg_import_system_collations? */
-#if defined(HAVE_LOCALE_T) && !defined(WIN32)
+#if !defined(WIN32)
 #define READ_LOCALE_A_OUTPUT
 #endif
 
diff --git a/src/backend/regex/regc_pg_locale.c b/src/backend/regex/regc_pg_locale.c
index 00ce735fdd..31e6300b5d 100644
--- a/src/backend/regex/regc_pg_locale.c
+++ b/src/backend/regex/regc_pg_locale.c
@@ -44,8 +44,7 @@
  * the platform's wchar_t representation matches what we do in pg_wchar
  * conversions.
  *
- * 3. Other collations are only supported on platforms that HAVE_LOCALE_T.
- * Here, we use the locale_t-extended forms of the <wctype.h> and <ctype.h>
+ * 3. Here, we use the locale_t-extended forms of the <wctype.h> and <ctype.h>
  * functions, under exactly the same cases as #2.
  *
  * There is one notable difference between cases 2 and 3: in the "default"
@@ -252,11 +251,6 @@ pg_set_regex_collation(Oid collation)
 	}
 	else
 	{
-		/*
-		 * NB: pg_newlocale_from_collation will fail if not HAVE_LOCALE_T; the
-		 * case of pg_regex_locale != 0 but not HAVE_LOCALE_T does not have to
-		 * be considered below.
-		 */
 		pg_regex_locale = pg_newlocale_from_collation(collation);
 
 		if (!pg_locale_deterministic(pg_regex_locale))
@@ -304,16 +298,12 @@ pg_wc_isdigit(pg_wchar c)
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isdigit((unsigned char) c));
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return iswdigit_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isdigit_l((unsigned char) c, pg_regex_locale->info.lt));
-#endif
 			break;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
@@ -340,16 +330,12 @@ pg_wc_isalpha(pg_wchar c)
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isalpha((unsigned char) c));
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return iswalpha_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isalpha_l((unsigned char) c, pg_regex_locale->info.lt));
-#endif
 			break;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
@@ -376,16 +362,12 @@ pg_wc_isalnum(pg_wchar c)
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isalnum((unsigned char) c));
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return iswalnum_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isalnum_l((unsigned char) c, pg_regex_locale->info.lt));
-#endif
 			break;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
@@ -421,16 +403,12 @@ pg_wc_isupper(pg_wchar c)
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isupper((unsigned char) c));
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return iswupper_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isupper_l((unsigned char) c, pg_regex_locale->info.lt));
-#endif
 			break;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
@@ -457,16 +435,12 @@ pg_wc_islower(pg_wchar c)
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					islower((unsigned char) c));
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return iswlower_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					islower_l((unsigned char) c, pg_regex_locale->info.lt));
-#endif
 			break;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
@@ -493,16 +467,12 @@ pg_wc_isgraph(pg_wchar c)
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isgraph((unsigned char) c));
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return iswgraph_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isgraph_l((unsigned char) c, pg_regex_locale->info.lt));
-#endif
 			break;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
@@ -529,16 +499,12 @@ pg_wc_isprint(pg_wchar c)
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isprint((unsigned char) c));
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return iswprint_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isprint_l((unsigned char) c, pg_regex_locale->info.lt));
-#endif
 			break;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
@@ -565,16 +531,12 @@ pg_wc_ispunct(pg_wchar c)
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					ispunct((unsigned char) c));
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return iswpunct_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					ispunct_l((unsigned char) c, pg_regex_locale->info.lt));
-#endif
 			break;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
@@ -601,16 +563,12 @@ pg_wc_isspace(pg_wchar c)
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isspace((unsigned char) c));
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return iswspace_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			return (c <= (pg_wchar) UCHAR_MAX &&
 					isspace_l((unsigned char) c, pg_regex_locale->info.lt));
-#endif
 			break;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
@@ -645,16 +603,12 @@ pg_wc_toupper(pg_wchar c)
 				return toupper((unsigned char) c);
 			return c;
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return towupper_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			if (c <= (pg_wchar) UCHAR_MAX)
 				return toupper_l((unsigned char) c, pg_regex_locale->info.lt);
-#endif
 			return c;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
@@ -689,16 +643,12 @@ pg_wc_tolower(pg_wchar c)
 				return tolower((unsigned char) c);
 			return c;
 		case PG_REGEX_LOCALE_WIDE_L:
-#ifdef HAVE_LOCALE_T
 			if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
 				return towlower_l((wint_t) c, pg_regex_locale->info.lt);
-#endif
 			/* FALL THRU */
 		case PG_REGEX_LOCALE_1BYTE_L:
-#ifdef HAVE_LOCALE_T
 			if (c <= (pg_wchar) UCHAR_MAX)
 				return tolower_l((unsigned char) c, pg_regex_locale->info.lt);
-#endif
 			return c;
 		case PG_REGEX_LOCALE_ICU:
 #ifdef USE_ICU
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index e6246dc44b..e27ea8ef97 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -1613,12 +1613,6 @@ u_strToTitle_default_BI(UChar *dest, int32_t destCapacity,
  * in multibyte character sets.  Note that in either case we are effectively
  * assuming that the database character encoding matches the encoding implied
  * by LC_CTYPE.
- *
- * If the system provides locale_t and associated functions (which are
- * standardized by Open Group's XBD), we can support collations that are
- * neither default nor C.  The code is written to handle both combinations
- * of have-wide-characters and have-locale_t, though it's rather unlikely
- * a platform would have the latter without the former.
  */
 
 /*
@@ -1696,11 +1690,9 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
 
 				for (curr_char = 0; workspace[curr_char] != 0; curr_char++)
 				{
-#ifdef HAVE_LOCALE_T
 					if (mylocale)
 						workspace[curr_char] = towlower_l(workspace[curr_char], mylocale->info.lt);
 					else
-#endif
 						workspace[curr_char] = towlower(workspace[curr_char]);
 				}
 
@@ -1729,11 +1721,9 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
 				 */
 				for (p = result; *p; p++)
 				{
-#ifdef HAVE_LOCALE_T
 					if (mylocale)
 						*p = tolower_l((unsigned char) *p, mylocale->info.lt);
 					else
-#endif
 						*p = pg_tolower((unsigned char) *p);
 				}
 			}
@@ -1818,11 +1808,9 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
 
 				for (curr_char = 0; workspace[curr_char] != 0; curr_char++)
 				{
-#ifdef HAVE_LOCALE_T
 					if (mylocale)
 						workspace[curr_char] = towupper_l(workspace[curr_char], mylocale->info.lt);
 					else
-#endif
 						workspace[curr_char] = towupper(workspace[curr_char]);
 				}
 
@@ -1851,11 +1839,9 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
 				 */
 				for (p = result; *p; p++)
 				{
-#ifdef HAVE_LOCALE_T
 					if (mylocale)
 						*p = toupper_l((unsigned char) *p, mylocale->info.lt);
 					else
-#endif
 						*p = pg_toupper((unsigned char) *p);
 				}
 			}
@@ -1941,7 +1927,6 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
 
 				for (curr_char = 0; workspace[curr_char] != 0; curr_char++)
 				{
-#ifdef HAVE_LOCALE_T
 					if (mylocale)
 					{
 						if (wasalnum)
@@ -1951,7 +1936,6 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
 						wasalnum = iswalnum_l(workspace[curr_char], mylocale->info.lt);
 					}
 					else
-#endif
 					{
 						if (wasalnum)
 							workspace[curr_char] = towlower(workspace[curr_char]);
@@ -1986,7 +1970,6 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
 				 */
 				for (p = result; *p; p++)
 				{
-#ifdef HAVE_LOCALE_T
 					if (mylocale)
 					{
 						if (wasalnum)
@@ -1996,7 +1979,6 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
 						wasalnum = isalnum_l((unsigned char) *p, mylocale->info.lt);
 					}
 					else
-#endif
 					{
 						if (wasalnum)
 							*p = pg_tolower((unsigned char) *p);
diff --git a/src/backend/utils/adt/like.c b/src/backend/utils/adt/like.c
index 33a2f46aab..62902caefa 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);
-#ifdef HAVE_LOCALE_T
 	else if (locale)
 		return tolower_l(c, locale->info.lt);
-#endif
 	else
 		return pg_tolower(c);
 }
diff --git a/src/backend/utils/adt/like_support.c b/src/backend/utils/adt/like_support.c
index 9b603d42f3..34e1b709ae 100644
--- a/src/backend/utils/adt/like_support.c
+++ b/src/backend/utils/adt/like_support.c
@@ -1509,10 +1509,8 @@ pattern_char_isalpha(char c, bool is_multibyte,
 	else if (locale && locale->provider == COLLPROVIDER_ICU)
 		return IS_HIGHBIT_SET(c) ||
 			(c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
-#ifdef HAVE_LOCALE_T
 	else if (locale && locale->provider == COLLPROVIDER_LIBC)
 		return isalpha_l((unsigned char) c, locale->info.lt);
-#endif
 	else
 		return isalpha((unsigned char) c);
 }
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index c8b36f3af2..83c5b960a7 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1420,7 +1420,6 @@ make_icu_collator(const char *iculocstr,
 
 
 /* simple subroutine for reporting errors from newlocale() */
-#ifdef HAVE_LOCALE_T
 static void
 report_newlocale_failure(const char *localename)
 {
@@ -1449,7 +1448,6 @@ report_newlocale_failure(const char *localename)
 			  errdetail("The operating system could not find any locale data for the locale name \"%s\".",
 						localename) : 0)));
 }
-#endif							/* HAVE_LOCALE_T */
 
 bool
 pg_locale_deterministic(pg_locale_t locale)
@@ -1466,10 +1464,6 @@ pg_locale_deterministic(pg_locale_t locale)
  * lifetime of the backend.  Thus, do not free the result with freelocale().
  *
  * As a special optimization, the default/database collation returns 0.
- * Callers should then revert to the non-locale_t-enabled code path.
- * 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.
  *
  * For simplicity, we always generate COLLATE + CTYPE even though we
  * might only need one of them.  Since this is called only once per session,
@@ -1515,7 +1509,6 @@ pg_newlocale_from_collation(Oid collid)
 
 		if (collform->collprovider == COLLPROVIDER_LIBC)
 		{
-#ifdef HAVE_LOCALE_T
 			const char *collcollate;
 			const char *collctype pg_attribute_unused();
 			locale_t	loc;
@@ -1566,12 +1559,6 @@ pg_newlocale_from_collation(Oid collid)
 			}
 
 			result.info.lt = loc;
-#else							/* not HAVE_LOCALE_T */
-			/* platform that doesn't support locale_t */
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("collation provider LIBC is not supported on this platform")));
-#endif							/* not HAVE_LOCALE_T */
 		}
 		else if (collform->collprovider == COLLPROVIDER_ICU)
 		{
@@ -1788,11 +1775,9 @@ pg_strncoll_libc_win32_utf8(const char *arg1, size_t len1, const char *arg2,
 	((LPWSTR) a2p)[r] = 0;
 
 	errno = 0;
-#ifdef HAVE_LOCALE_T
 	if (locale)
 		result = wcscoll_l((LPWSTR) a1p, (LPWSTR) a2p, locale->info.lt);
 	else
-#endif
 		result = wcscoll((LPWSTR) a1p, (LPWSTR) a2p);
 	if (result == 2147483647)	/* _NLSCMPERROR; missing from mingw headers */
 		ereport(ERROR,
@@ -1831,14 +1816,7 @@ pg_strcoll_libc(const char *arg1, const char *arg2, pg_locale_t locale)
 	else
 #endif							/* WIN32 */
 	if (locale)
-	{
-#ifdef HAVE_LOCALE_T
 		result = strcoll_l(arg1, arg2, locale->info.lt);
-#else
-		/* shouldn't happen */
-		elog(ERROR, "unsupported collprovider: %c", locale->provider);
-#endif
-	}
 	else
 		result = strcoll(arg1, arg2);
 
@@ -2065,11 +2043,9 @@ pg_strxfrm_libc(char *dest, const char *src, size_t destsize,
 	Assert(!locale || locale->provider == COLLPROVIDER_LIBC);
 
 #ifdef TRUST_STRXFRM
-#ifdef HAVE_LOCALE_T
 	if (locale)
 		return strxfrm_l(dest, src, destsize, locale->info.lt);
 	else
-#endif
 		return strxfrm(dest, src, destsize);
 #else
 	/* shouldn't happen */
@@ -2955,7 +2931,6 @@ wchar2char(char *to, const wchar_t *from, size_t tolen, pg_locale_t locale)
 	}
 	else
 	{
-#ifdef HAVE_LOCALE_T
 #ifdef HAVE_WCSTOMBS_L
 		/* Use wcstombs_l for nondefault locales */
 		result = wcstombs_l(to, from, tolen, locale->info.lt);
@@ -2967,11 +2942,6 @@ wchar2char(char *to, const wchar_t *from, size_t tolen, pg_locale_t locale)
 
 		uselocale(save_locale);
 #endif							/* HAVE_WCSTOMBS_L */
-#else							/* !HAVE_LOCALE_T */
-		/* Can't have locale != 0 without HAVE_LOCALE_T */
-		elog(ERROR, "wcstombs_l is not available");
-		result = 0;				/* keep compiler quiet */
-#endif							/* HAVE_LOCALE_T */
 	}
 
 	return result;
@@ -3032,7 +3002,6 @@ char2wchar(wchar_t *to, size_t tolen, const char *from, size_t fromlen,
 		}
 		else
 		{
-#ifdef HAVE_LOCALE_T
 #ifdef HAVE_MBSTOWCS_L
 			/* Use mbstowcs_l for nondefault locales */
 			result = mbstowcs_l(to, str, tolen, locale->info.lt);
@@ -3044,11 +3013,6 @@ char2wchar(wchar_t *to, size_t tolen, const char *from, size_t fromlen,
 
 			uselocale(save_locale);
 #endif							/* HAVE_MBSTOWCS_L */
-#else							/* !HAVE_LOCALE_T */
-			/* Can't have locale != 0 without HAVE_LOCALE_T */
-			elog(ERROR, "mbstowcs_l is not available");
-			result = 0;			/* keep compiler quiet */
-#endif							/* HAVE_LOCALE_T */
 		}
 
 		pfree(str);
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 6d572c3820..c33fc4b8cd 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -298,9 +298,6 @@
 /* Define to 1 if you have the `zstd' library (-lzstd). */
 #undef HAVE_LIBZSTD
 
-/* Define to 1 if the system has the type `locale_t'. */
-#undef HAVE_LOCALE_T
-
 /* Define to 1 if `long int' works and is 64 bits. */
 #undef HAVE_LONG_INT_64
 
diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index b957d5c598..dee85fa3b2 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -458,6 +458,9 @@ extern int	_pglstat64(const char *name, struct stat *buf);
 #define wcstombs_l _wcstombs_l
 #define mbstowcs_l _mbstowcs_l
 
+#define HAVE_WCSTOMBS_L 1
+#define HAVE_MBSTOWCS_L 1
+
 /*
  * Versions of libintl >= 0.18? try to replace setlocale() with a macro
  * to their own versions.  Remove the macro, if it exists, because it
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index e2a7243542..6447bea8e0 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -67,9 +67,7 @@ extern void cache_locale_time(void);
 
 
 /*
- * We define our own wrapper around locale_t so we can keep the same
- * function signatures for all builds, while not having to create a
- * fake version of the standard type locale_t in the global namespace.
+ * We use a discriminated union to hold either a locale_t or an ICU collator.
  * pg_locale_t is occasionally checked for truth, so make it a pointer.
  */
 struct pg_locale_struct
@@ -78,9 +76,7 @@ struct pg_locale_struct
 	bool		deterministic;
 	union
 	{
-#ifdef HAVE_LOCALE_T
 		locale_t	lt;
-#endif
 #ifdef USE_ICU
 		struct
 		{
@@ -88,7 +84,6 @@ struct pg_locale_struct
 			UCollator  *ucol;
 		}			icu;
 #endif
-		int			dummy;		/* in case we have neither LOCALE_T nor ICU */
 	}			info;
 };
 
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index b6d31c3583..bf708f15b3 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -296,7 +296,6 @@ sub GenerateFiles
 		HAVE_LIBXSLT => undef,
 		HAVE_LIBZ => $self->{options}->{zlib} ? 1 : undef,
 		HAVE_LIBZSTD => undef,
-		HAVE_LOCALE_T => 1,
 		HAVE_LONG_INT_64 => undef,
 		HAVE_LONG_LONG_INT_64 => 1,
 		HAVE_MBARRIER_H => undef,
-- 
2.39.2



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: check_strxfrm_bug()
@ 2023-07-01 16:25  Noah Misch <[email protected]>
  parent: Thomas Munro <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Noah Misch @ 2023-07-01 16:25 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; pgsql-hackers

On Wed, Jun 28, 2023 at 01:02:21PM +1200, Thomas Munro wrote:
> On Wed, Jun 28, 2023 at 11:03 AM Thomas Munro <[email protected]> wrote:
> > The GCC build farm has just received some SPARC hardware new enough to
> > run modern Solaris (hostname gcc106), so if wrasse were moved over
> > there we could finally assume all systems have POSIX 2008 (AKA
> > SUSv4)'s locale_t.
> 
> That would look something like this.

This removes thirty-eight ifdefs, most of them located in the middle of
function bodies.  That's far more beneficial than most proposals to raise
minimum requirements.  +1 for revoking support for wrasse's OS version.
(wrasse wouldn't move, but it would stop testing v17+.)






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: check_strxfrm_bug()
@ 2023-07-03 01:49  Thomas Munro <[email protected]>
  parent: Noah Misch <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Thomas Munro @ 2023-07-03 01:49 UTC (permalink / raw)
  To: Noah Misch <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; pgsql-hackers

On Sun, Jul 2, 2023 at 4:25 AM Noah Misch <[email protected]> wrote:
> On Wed, Jun 28, 2023 at 01:02:21PM +1200, Thomas Munro wrote:
> > On Wed, Jun 28, 2023 at 11:03 AM Thomas Munro <[email protected]> wrote:
> > > The GCC build farm has just received some SPARC hardware new enough to
> > > run modern Solaris (hostname gcc106), so if wrasse were moved over
> > > there we could finally assume all systems have POSIX 2008 (AKA
> > > SUSv4)'s locale_t.
> >
> > That would look something like this.
>
> This removes thirty-eight ifdefs, most of them located in the middle of
> function bodies.  That's far more beneficial than most proposals to raise
> minimum requirements.  +1 for revoking support for wrasse's OS version.
> (wrasse wouldn't move, but it would stop testing v17+.)

Great.  It sounds like I should wait a few days for any other feedback
and then push this patch.  Thanks for looking.






^ permalink  raw  reply  [nested|flat] 6+ messages in thread


end of thread, other threads:[~2023-07-03 01:49 UTC | newest]

Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-03-13 07:58 [PATCH v49 2/7] Add conditional lock feature to dshash Kyotaro Horiguchi <[email protected]>
2023-06-12 08:48 Re: check_strxfrm_bug() Heikki Linnakangas <[email protected]>
2023-06-27 23:03 ` Re: check_strxfrm_bug() Thomas Munro <[email protected]>
2023-06-28 01:02   ` Re: check_strxfrm_bug() Thomas Munro <[email protected]>
2023-07-01 16:25     ` Re: check_strxfrm_bug() Noah Misch <[email protected]>
2023-07-03 01:49       ` Re: check_strxfrm_bug() Thomas Munro <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox