On Wed, 2024-08-14 at 16:30 -0700, Jeff Davis wrote:
> On Thu, 2024-08-08 at 12:24 -0700, Jeff Davis wrote:
> > The collation cache, which maps collation oids to pg_locale_t
> > objects,
> > has a few longstanding issues:
>
> Here's a patch set v2.
Updated and rebased.
Regards,
Jeff Davis
From 224470bc4d0660dc11940f5595031eecb0319d62 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Wed, 7 Aug 2024 11:05:46 -0700
Subject: [PATCH v4 1/7] Tighten up make_libc_collator() and
make_icu_collator().
Return the result rather than using an out parameter, and make it the
caller's responsibility to copy it into the right context. Ensure that
no paths leak a collator.
The function make_icu_collator() doesn't have any external callers, so
change it to be static. Also, when re-opening with rules, use a
try/finally block to avoid leaking the collator.
In make_libc_collator(), if the first newlocale() succeeds and the
second one fails, close the first locale_t object.
Discussion: https://postgr.es/m/54d20e812bd6c3e44c10eddcd757ec494ebf1803.ca...@j-davis.com
---
src/backend/utils/adt/pg_locale.c | 126 +++++++++++++++++++-----------
src/include/utils/pg_locale.h | 4 -
2 files changed, 80 insertions(+), 50 deletions(-)
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 5bef1b113a8..12ba5726f77 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1297,14 +1297,15 @@ report_newlocale_failure(const char *localename)
}
/*
- * Initialize the locale_t field.
+ * Create a locale_t with the given collation and ctype.
*
- * The "C" and "POSIX" locales are not actually handled by libc, so set the
- * locale_t to zero in that case.
+ * The "C" and "POSIX" locales are not actually handled by libc, so return
+ * NULL.
+ *
+ * Ensure that no path leaks a locale_t.
*/
-static void
-make_libc_collator(const char *collate, const char *ctype,
- pg_locale_t result)
+static locale_t
+make_libc_collator(const char *collate, const char *ctype)
{
locale_t loc = 0;
@@ -1343,7 +1344,11 @@ make_libc_collator(const char *collate, const char *ctype,
errno = 0;
loc = newlocale(LC_CTYPE_MASK, ctype, loc1);
if (!loc)
+ {
+ if (loc1)
+ freelocale(loc1);
report_newlocale_failure(ctype);
+ }
}
else
loc = loc1;
@@ -1360,60 +1365,78 @@ make_libc_collator(const char *collate, const char *ctype,
#endif
}
- result->info.lt = loc;
+ return loc;
}
-void
-make_icu_collator(const char *iculocstr,
- const char *icurules,
- struct pg_locale_struct *resultp)
-{
+/*
+ * Create a UCollator with the given locale string and rules.
+ *
+ * Ensure that no path leaks a UCollator.
+ */
#ifdef USE_ICU
- UCollator *collator;
-
- collator = pg_ucol_open(iculocstr);
-
- /*
- * If rules are specified, we extract the rules of the standard collation,
- * add our own rules, and make a new collator with the combined rules.
- */
- if (icurules)
+static UCollator *
+make_icu_collator(const char *iculocstr, const char *icurules)
+{
+ if (!icurules)
{
- const UChar *default_rules;
- UChar *agg_rules;
+ /* simple case without rules */
+ return pg_ucol_open(iculocstr);
+ }
+ else
+ {
+ UCollator *collator_std_rules;
+ UCollator *collator_all_rules;
+ const UChar *std_rules;
UChar *my_rules;
- UErrorCode status;
+ UChar *all_rules;
int32_t length;
+ int32_t total;
+ UErrorCode status;
- default_rules = ucol_getRules(collator, &length);
+ /*
+ * If rules are specified, we extract the rules of the standard
+ * collation, add our own rules, and make a new collator with the
+ * combined rules.
+ */
icu_to_uchar(&my_rules, icurules, strlen(icurules));
- agg_rules = palloc_array(UChar, u_strlen(default_rules) + u_strlen(my_rules) + 1);
- u_strcpy(agg_rules, default_rules);
- u_strcat(agg_rules, my_rules);
+ collator_std_rules = pg_ucol_open(iculocstr);
- ucol_close(collator);
+ std_rules = ucol_getRules(collator_std_rules, &length);
+
+ total = u_strlen(std_rules) + u_strlen(my_rules) + 1;
+
+ /* avoid leaking collator on OOM */
+ all_rules = palloc_extended(sizeof(UChar) * total, MCXT_ALLOC_NO_OOM);
+ if (!all_rules)
+ {
+ ucol_close(collator_std_rules);
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+ }
+
+ u_strcpy(all_rules, std_rules);
+ u_strcat(all_rules, my_rules);
+
+ ucol_close(collator_std_rules);
status = U_ZERO_ERROR;
- collator = ucol_openRules(agg_rules, u_strlen(agg_rules),
- UCOL_DEFAULT, UCOL_DEFAULT_STRENGTH, NULL, &status);
+ collator_all_rules = ucol_openRules(all_rules, u_strlen(all_rules),
+ UCOL_DEFAULT, UCOL_DEFAULT_STRENGTH,
+ NULL, &status);
if (U_FAILURE(status))
+ {
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("could not open collator for locale \"%s\" with rules \"%s\": %s",
iculocstr, icurules, u_errorName(status))));
- }
+ }
- /* We will leak this string if the caller errors later :-( */
- resultp->info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr);
- resultp->info.icu.ucol = collator;
-#else /* not USE_ICU */
- /* could get here if a collation was created by a build with ICU */
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("ICU is not supported in this build")));
-#endif /* not USE_ICU */
+ return collator_all_rules;
+ }
}
+#endif /* not USE_ICU */
/*
* Initialize default_locale with database locale settings.
@@ -1424,7 +1447,6 @@ init_database_collation(void)
HeapTuple tup;
Form_pg_database dbform;
Datum datum;
- bool isnull;
/* Fetch our pg_database row normally, via syscache */
tup = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId));
@@ -1449,8 +1471,10 @@ init_database_collation(void)
}
else if (dbform->datlocprovider == COLLPROVIDER_ICU)
{
+#ifdef USE_ICU
char *datlocale;
char *icurules;
+ bool isnull;
datum = SysCacheGetAttrNotNull(DATABASEOID, tup, Anum_pg_database_datlocale);
datlocale = TextDatumGetCString(datum);
@@ -1464,7 +1488,14 @@ init_database_collation(void)
else
icurules = NULL;
- make_icu_collator(datlocale, icurules, &default_locale);
+ default_locale.info.icu.locale = MemoryContextStrdup(TopMemoryContext, datlocale);
+ default_locale.info.icu.ucol = make_icu_collator(datlocale, icurules);
+#else /* not USE_ICU */
+ /* could get here if a collation was created by a build with ICU */
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("ICU is not supported in this build")));
+#endif /* not USE_ICU */
}
else
{
@@ -1483,7 +1514,7 @@ init_database_collation(void)
default_locale.ctype_is_c = (strcmp(datctype, "C") == 0) ||
(strcmp(datctype, "POSIX") == 0);
- make_libc_collator(datcollate, datctype, &default_locale);
+ default_locale.info.lt = make_libc_collator(datcollate, datctype);
}
default_locale.provider = dbform->datlocprovider;
@@ -1572,7 +1603,7 @@ pg_newlocale_from_collation(Oid collid)
result.ctype_is_c = (strcmp(collctype, "C") == 0) ||
(strcmp(collctype, "POSIX") == 0);
- make_libc_collator(collcollate, collctype, &result);
+ result.info.lt = make_libc_collator(collcollate, collctype);
}
else if (collform->collprovider == COLLPROVIDER_ICU)
{
@@ -1591,7 +1622,8 @@ pg_newlocale_from_collation(Oid collid)
else
icurules = NULL;
- make_icu_collator(iculocstr, icurules, &result);
+ result.info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr);
+ result.info.icu.ucol = make_icu_collator(iculocstr, icurules);
}
datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collversion,
@@ -2500,6 +2532,8 @@ builtin_validate_locale(int encoding, const char *locale)
/*
* Wrapper around ucol_open() to handle API differences for older ICU
* versions.
+ *
+ * Ensure that no path leaks a UCollator.
*/
static UCollator *
pg_ucol_open(const char *loc_str)
diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h
index faae868bfcc..c2d95411e0a 100644
--- a/src/include/utils/pg_locale.h
+++ b/src/include/utils/pg_locale.h
@@ -104,10 +104,6 @@ struct pg_locale_struct
typedef struct pg_locale_struct *pg_locale_t;
-extern void make_icu_collator(const char *iculocstr,
- const char *icurules,
- struct pg_locale_struct *resultp);
-
extern void init_database_collation(void);
extern pg_locale_t pg_newlocale_from_collation(Oid collid);
--
2.34.1
From eccc4a4a83069c6a14465b4a9239a4d759aaa2a8 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Wed, 18 Sep 2024 15:53:56 -0700
Subject: [PATCH v4 2/7] create_pg_locale
---
src/backend/utils/adt/pg_locale.c | 310 +++++++++++++++---------------
1 file changed, 155 insertions(+), 155 deletions(-)
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 12ba5726f77..1dec00b55ed 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1227,45 +1227,6 @@ IsoLocaleName(const char *winlocname)
#endif /* WIN32 && LC_MESSAGES */
-/*
- * Cache mechanism for collation information.
- *
- * Note that we currently lack any way to flush the cache. Since we don't
- * support ALTER COLLATION, this is OK. The worst case is that someone
- * drops a collation, and a useless cache entry hangs around in existing
- * backends.
- */
-static collation_cache_entry *
-lookup_collation_cache(Oid collation)
-{
- collation_cache_entry *cache_entry;
- bool found;
-
- Assert(OidIsValid(collation));
- Assert(collation != DEFAULT_COLLATION_OID);
-
- if (CollationCache == NULL)
- {
- CollationCacheContext = AllocSetContextCreate(TopMemoryContext,
- "collation cache",
- ALLOCSET_DEFAULT_SIZES);
- CollationCache = collation_cache_create(CollationCacheContext,
- 16, NULL);
- }
-
- cache_entry = collation_cache_insert(CollationCache, collation, &found);
- if (!found)
- {
- /*
- * Make sure cache entry is marked invalid, in case we fail before
- * setting things.
- */
- cache_entry->locale = 0;
- }
-
- return cache_entry;
-}
-
/* simple subroutine for reporting errors from newlocale() */
static void
report_newlocale_failure(const char *localename)
@@ -1530,153 +1491,192 @@ init_database_collation(void)
}
/*
- * Create a pg_locale_t from a collation OID. Results are cached for the
- * lifetime of the backend. Thus, do not free the result with freelocale().
- *
- * For simplicity, we always generate COLLATE + CTYPE even though we
- * might only need one of them. Since this is called only once per session,
- * it shouldn't cost much.
+ * Create and initialize a pg_locale_t. Be careful to check for errors before
+ * allocating memory.
*/
-pg_locale_t
-pg_newlocale_from_collation(Oid collid)
+static pg_locale_t
+create_pg_locale(Oid collid)
{
- collation_cache_entry *cache_entry;
-
- if (collid == DEFAULT_COLLATION_OID)
- return &default_locale;
+ /* We haven't computed this yet in this session, so do it */
+ HeapTuple tp;
+ Form_pg_collation collform;
+ pg_locale_t result;
+ Datum datum;
+ bool isnull;
- if (!OidIsValid(collid))
+ tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(collid));
+ if (!HeapTupleIsValid(tp))
elog(ERROR, "cache lookup failed for collation %u", collid);
+ collform = (Form_pg_collation) GETSTRUCT(tp);
- if (last_collation_cache_oid == collid)
- return last_collation_cache_locale;
-
- cache_entry = lookup_collation_cache(collid);
-
- if (cache_entry->locale == 0)
+ /* compare version in catalog to version from provider */
+ datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collversion,
+ &isnull);
+ if (!isnull)
{
- /* We haven't computed this yet in this session, so do it */
- HeapTuple tp;
- Form_pg_collation collform;
- struct pg_locale_struct result;
- pg_locale_t resultp;
- Datum datum;
- bool isnull;
+ char *actual_versionstr;
+ char *collversionstr;
- tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(collid));
- if (!HeapTupleIsValid(tp))
- elog(ERROR, "cache lookup failed for collation %u", collid);
- collform = (Form_pg_collation) GETSTRUCT(tp);
+ collversionstr = TextDatumGetCString(datum);
- /* We'll fill in the result struct locally before allocating memory */
- memset(&result, 0, sizeof(result));
- result.provider = collform->collprovider;
- result.deterministic = collform->collisdeterministic;
+ if (collform->collprovider == COLLPROVIDER_LIBC)
+ datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collcollate);
+ else
+ datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_colllocale);
- if (collform->collprovider == COLLPROVIDER_BUILTIN)
+ actual_versionstr = get_collation_actual_version(collform->collprovider,
+ TextDatumGetCString(datum));
+ if (!actual_versionstr)
{
- const char *locstr;
+ /*
+ * This could happen when specifying a version in CREATE
+ * COLLATION but the provider does not support versioning, or
+ * manually creating a mess in the catalogs.
+ */
+ ereport(ERROR,
+ (errmsg("collation \"%s\" has no actual version, but a version was recorded",
+ NameStr(collform->collname))));
+ }
- datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_colllocale);
- locstr = TextDatumGetCString(datum);
+ if (strcmp(actual_versionstr, collversionstr) != 0)
+ ereport(WARNING,
+ (errmsg("collation \"%s\" has version mismatch",
+ NameStr(collform->collname)),
+ errdetail("The collation in the database was created using version %s, "
+ "but the operating system provides version %s.",
+ collversionstr, actual_versionstr),
+ errhint("Rebuild all objects affected by this collation and run "
+ "ALTER COLLATION %s REFRESH VERSION, "
+ "or build PostgreSQL with the right library version.",
+ quote_qualified_identifier(get_namespace_name(collform->collnamespace),
+ NameStr(collform->collname)))));
+ }
- result.collate_is_c = true;
- result.ctype_is_c = (strcmp(locstr, "C") == 0);
+ if (collform->collprovider == COLLPROVIDER_BUILTIN)
+ {
+ const char *locstr;
- builtin_validate_locale(GetDatabaseEncoding(), locstr);
+ datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_colllocale);
+ locstr = TextDatumGetCString(datum);
- result.info.builtin.locale = MemoryContextStrdup(TopMemoryContext,
- locstr);
- }
- else if (collform->collprovider == COLLPROVIDER_LIBC)
- {
- const char *collcollate;
- const char *collctype;
+ builtin_validate_locale(GetDatabaseEncoding(), locstr);
- datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collcollate);
- collcollate = TextDatumGetCString(datum);
- datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collctype);
- collctype = TextDatumGetCString(datum);
+ result = MemoryContextAllocZero(TopMemoryContext,
+ sizeof(struct pg_locale_struct));
- result.collate_is_c = (strcmp(collcollate, "C") == 0) ||
- (strcmp(collcollate, "POSIX") == 0);
- result.ctype_is_c = (strcmp(collctype, "C") == 0) ||
- (strcmp(collctype, "POSIX") == 0);
+ result->provider = collform->collprovider;
+ result->deterministic = collform->collisdeterministic;
+ result->collate_is_c = true;
+ result->ctype_is_c = (strcmp(locstr, "C") == 0);
+ result->info.builtin.locale = MemoryContextStrdup(TopMemoryContext,
+ locstr);
+ }
+ else if (collform->collprovider == COLLPROVIDER_LIBC)
+ {
+ const char *collcollate;
+ const char *collctype;
+ locale_t locale;
+
+ datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collcollate);
+ collcollate = TextDatumGetCString(datum);
+ datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collctype);
+ collctype = TextDatumGetCString(datum);
+
+ locale = make_libc_collator(collcollate, collctype);
+
+ result = MemoryContextAllocZero(TopMemoryContext,
+ sizeof(struct pg_locale_struct));
+
+ result->provider = collform->collprovider;
+ result->deterministic = collform->collisdeterministic;
+ result->collate_is_c = (strcmp(collcollate, "C") == 0) ||
+ (strcmp(collcollate, "POSIX") == 0);
+ result->ctype_is_c = (strcmp(collctype, "C") == 0) ||
+ (strcmp(collctype, "POSIX") == 0);
+ result->info.lt = locale;
+ }
+ else if (collform->collprovider == COLLPROVIDER_ICU)
+ {
+ const char *iculocstr;
+ const char *icurules;
+ UCollator *collator;
- result.info.lt = make_libc_collator(collcollate, collctype);
- }
- else if (collform->collprovider == COLLPROVIDER_ICU)
- {
- const char *iculocstr;
- const char *icurules;
+ datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_colllocale);
+ iculocstr = TextDatumGetCString(datum);
- datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_colllocale);
- iculocstr = TextDatumGetCString(datum);
+ datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collicurules, &isnull);
+ if (!isnull)
+ icurules = TextDatumGetCString(datum);
+ else
+ icurules = NULL;
- result.collate_is_c = false;
- result.ctype_is_c = false;
+ collator = make_icu_collator(iculocstr, icurules);
- datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collicurules, &isnull);
- if (!isnull)
- icurules = TextDatumGetCString(datum);
- else
- icurules = NULL;
+ result = MemoryContextAllocZero(TopMemoryContext,
+ sizeof(struct pg_locale_struct));
- result.info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr);
- result.info.icu.ucol = make_icu_collator(iculocstr, icurules);
- }
+ result->provider = collform->collprovider;
+ result->deterministic = collform->collisdeterministic;
+ result->collate_is_c = false;
+ result->ctype_is_c = false;
+ result->info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr);
+ result->info.icu.ucol = collator;
+ }
- datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collversion,
- &isnull);
- if (!isnull)
- {
- char *actual_versionstr;
- char *collversionstr;
+ ReleaseSysCache(tp);
- collversionstr = TextDatumGetCString(datum);
+ return result;
+}
- if (collform->collprovider == COLLPROVIDER_LIBC)
- datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collcollate);
- else
- datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_colllocale);
+/*
+ * Create or retrieve a pg_locale_t for the given collation OID. Results are
+ * cached for the lifetime of the backend.
+ */
+pg_locale_t
+pg_newlocale_from_collation(Oid collid)
+{
+ collation_cache_entry *cache_entry;
+ bool found;
- actual_versionstr = get_collation_actual_version(collform->collprovider,
- TextDatumGetCString(datum));
- if (!actual_versionstr)
- {
- /*
- * This could happen when specifying a version in CREATE
- * COLLATION but the provider does not support versioning, or
- * manually creating a mess in the catalogs.
- */
- ereport(ERROR,
- (errmsg("collation \"%s\" has no actual version, but a version was recorded",
- NameStr(collform->collname))));
- }
+ if (collid == DEFAULT_COLLATION_OID)
+ return &default_locale;
- if (strcmp(actual_versionstr, collversionstr) != 0)
- ereport(WARNING,
- (errmsg("collation \"%s\" has version mismatch",
- NameStr(collform->collname)),
- errdetail("The collation in the database was created using version %s, "
- "but the operating system provides version %s.",
- collversionstr, actual_versionstr),
- errhint("Rebuild all objects affected by this collation and run "
- "ALTER COLLATION %s REFRESH VERSION, "
- "or build PostgreSQL with the right library version.",
- quote_qualified_identifier(get_namespace_name(collform->collnamespace),
- NameStr(collform->collname)))));
- }
+ if (!OidIsValid(collid))
+ elog(ERROR, "cache lookup failed for collation %u", collid);
- ReleaseSysCache(tp);
+ if (last_collation_cache_oid == collid)
+ return last_collation_cache_locale;
- /* We'll keep the pg_locale_t structures in TopMemoryContext */
- resultp = MemoryContextAlloc(TopMemoryContext, sizeof(*resultp));
- *resultp = result;
+ /*
+ * Cache mechanism for collation information.
+ *
+ * Note that we currently lack any way to flush the cache. Since we don't
+ * support ALTER COLLATION, this is OK. The worst case is that someone
+ * drops a collation, and a useless cache entry hangs around in existing
+ * backends.
+ */
+ if (CollationCache == NULL)
+ {
+ CollationCacheContext = AllocSetContextCreate(TopMemoryContext,
+ "collation cache",
+ ALLOCSET_DEFAULT_SIZES);
+ CollationCache = collation_cache_create(CollationCacheContext,
+ 16, NULL);
+ }
- cache_entry->locale = resultp;
+ cache_entry = collation_cache_insert(CollationCache, collid, &found);
+ if (!found)
+ {
+ /*
+ * Make sure cache entry is marked invalid, in case we fail before
+ * setting things.
+ */
+ cache_entry->locale = 0;
}
+ if (cache_entry->locale == 0)
+ cache_entry->locale = create_pg_locale(collid);
+
last_collation_cache_oid = collid;
last_collation_cache_locale = cache_entry->locale;
--
2.34.1
From fca0efa184971f9780b356039aa3ed08a7445524 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Wed, 18 Sep 2024 15:55:37 -0700
Subject: [PATCH v4 3/7] CollationCacheContext
---
src/backend/utils/adt/pg_locale.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 1dec00b55ed..d3d9c3920e6 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -1495,7 +1495,7 @@ init_database_collation(void)
* allocating memory.
*/
static pg_locale_t
-create_pg_locale(Oid collid)
+create_pg_locale(MemoryContext context, Oid collid)
{
/* We haven't computed this yet in this session, so do it */
HeapTuple tp;
@@ -1561,15 +1561,15 @@ create_pg_locale(Oid collid)
builtin_validate_locale(GetDatabaseEncoding(), locstr);
- result = MemoryContextAllocZero(TopMemoryContext,
+ result = MemoryContextAllocZero(context,
sizeof(struct pg_locale_struct));
result->provider = collform->collprovider;
result->deterministic = collform->collisdeterministic;
result->collate_is_c = true;
result->ctype_is_c = (strcmp(locstr, "C") == 0);
- result->info.builtin.locale = MemoryContextStrdup(TopMemoryContext,
- locstr);
+ result->info.builtin.locale = MemoryContextStrdup(context,
+ locstr);
}
else if (collform->collprovider == COLLPROVIDER_LIBC)
{
@@ -1584,7 +1584,7 @@ create_pg_locale(Oid collid)
locale = make_libc_collator(collcollate, collctype);
- result = MemoryContextAllocZero(TopMemoryContext,
+ result = MemoryContextAllocZero(context,
sizeof(struct pg_locale_struct));
result->provider = collform->collprovider;
@@ -1612,14 +1612,14 @@ create_pg_locale(Oid collid)
collator = make_icu_collator(iculocstr, icurules);
- result = MemoryContextAllocZero(TopMemoryContext,
+ result = MemoryContextAllocZero(context,
sizeof(struct pg_locale_struct));
result->provider = collform->collprovider;
result->deterministic = collform->collisdeterministic;
result->collate_is_c = false;
result->ctype_is_c = false;
- result->info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr);
+ result->info.icu.locale = MemoryContextStrdup(context, iculocstr);
result->info.icu.ucol = collator;
}
@@ -1675,7 +1675,7 @@ pg_newlocale_from_collation(Oid collid)
}
if (cache_entry->locale == 0)
- cache_entry->locale = create_pg_locale(collid);
+ cache_entry->locale = create_pg_locale(CollationCacheContext, collid);
last_collation_cache_oid = collid;
last_collation_cache_locale = cache_entry->locale;
--
2.34.1
From 5ae3b1be6489617a1639141749c31d2f4419a676 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Wed, 18 Sep 2024 16:55:42 -0700
Subject: [PATCH v4 4/7] resource owners
---
src/backend/utils/adt/pg_locale.c | 74 ++++++++++++++++++++++++++++++-
1 file changed, 72 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index d3d9c3920e6..9d1d71f1561 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -66,6 +66,7 @@
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/pg_locale.h"
+#include "utils/resowner.h"
#include "utils/syscache.h"
#ifdef USE_ICU
@@ -148,6 +149,12 @@ typedef struct
#define SH_DEFINE
#include "lib/simplehash.h"
+/*
+ * Collator objects (UCollator for ICU or locale_t for libc) are allocated in
+ * an external library, so track them using a resource owner.
+ */
+static ResourceOwner CollationCacheOwner = NULL;
+
static MemoryContext CollationCacheContext = NULL;
static collation_cache_hash *CollationCache = NULL;
@@ -179,8 +186,35 @@ static int32_t uchar_convert(UConverter *converter,
const char *src, int32_t srclen);
static void icu_set_collation_attributes(UCollator *collator, const char *loc,
UErrorCode *status);
+
+static void ResourceOwnerRememberUCollator(ResourceOwner owner,
+ UCollator *collator);
+static void ResOwnerReleaseUCollator(Datum val);
+
+static const ResourceOwnerDesc UCollatorResourceKind =
+{
+ .name = "UCollator reference",
+ .release_phase = RESOURCE_RELEASE_AFTER_LOCKS,
+ .release_priority = RELEASE_PRIO_LAST,
+ .ReleaseResource = ResOwnerReleaseUCollator,
+ .DebugPrint = NULL /* the default message is fine */
+};
#endif
+static void ResourceOwnerRememberLocaleT(ResourceOwner owner,
+ locale_t locale);
+static void ResOwnerReleaseLocaleT(Datum val);
+
+static const ResourceOwnerDesc LocaleTResourceKind =
+{
+ .name = "locale_t reference",
+ .release_phase = RESOURCE_RELEASE_AFTER_LOCKS,
+ .release_priority = RELEASE_PRIO_LAST,
+ .ReleaseResource = ResOwnerReleaseLocaleT,
+ .DebugPrint = NULL /* the default message is fine */
+};
+
+
/*
* POSIX doesn't define _l-variants of these functions, but several systems
* have them. We provide our own replacements here.
@@ -1257,6 +1291,20 @@ report_newlocale_failure(const char *localename)
localename) : 0)));
}
+static void
+ResourceOwnerRememberLocaleT(ResourceOwner owner, locale_t locale)
+{
+ ResourceOwnerRemember(owner, PointerGetDatum(locale),
+ &LocaleTResourceKind);
+}
+
+static void
+ResOwnerReleaseLocaleT(Datum val)
+{
+ locale_t locale = (locale_t) DatumGetPointer(val);
+ freelocale(locale);
+}
+
/*
* Create a locale_t with the given collation and ctype.
*
@@ -1335,6 +1383,20 @@ make_libc_collator(const char *collate, const char *ctype)
* Ensure that no path leaks a UCollator.
*/
#ifdef USE_ICU
+static void
+ResourceOwnerRememberUCollator(ResourceOwner owner, UCollator *collator)
+{
+ ResourceOwnerRemember(owner, PointerGetDatum(collator),
+ &UCollatorResourceKind);
+}
+
+static void
+ResOwnerReleaseUCollator(Datum val)
+{
+ UCollator *collator = (UCollator *) DatumGetPointer(val);
+ ucol_close(collator);
+}
+
static UCollator *
make_icu_collator(const char *iculocstr, const char *icurules)
{
@@ -1495,7 +1557,7 @@ init_database_collation(void)
* allocating memory.
*/
static pg_locale_t
-create_pg_locale(MemoryContext context, Oid collid)
+create_pg_locale(MemoryContext context, ResourceOwner owner, Oid collid)
{
/* We haven't computed this yet in this session, so do it */
HeapTuple tp;
@@ -1582,7 +1644,10 @@ create_pg_locale(MemoryContext context, Oid collid)
datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collctype);
collctype = TextDatumGetCString(datum);
+ ResourceOwnerEnlarge(owner);
locale = make_libc_collator(collcollate, collctype);
+ if (locale)
+ ResourceOwnerRememberLocaleT(owner, locale);
result = MemoryContextAllocZero(context,
sizeof(struct pg_locale_struct));
@@ -1610,7 +1675,9 @@ create_pg_locale(MemoryContext context, Oid collid)
else
icurules = NULL;
+ ResourceOwnerEnlarge(owner);
collator = make_icu_collator(iculocstr, icurules);
+ ResourceOwnerRememberUCollator(owner, collator);
result = MemoryContextAllocZero(context,
sizeof(struct pg_locale_struct));
@@ -1657,6 +1724,7 @@ pg_newlocale_from_collation(Oid collid)
*/
if (CollationCache == NULL)
{
+ CollationCacheOwner = ResourceOwnerCreate(NULL, "collation cache");
CollationCacheContext = AllocSetContextCreate(TopMemoryContext,
"collation cache",
ALLOCSET_DEFAULT_SIZES);
@@ -1675,7 +1743,9 @@ pg_newlocale_from_collation(Oid collid)
}
if (cache_entry->locale == 0)
- cache_entry->locale = create_pg_locale(CollationCacheContext, collid);
+ cache_entry->locale = create_pg_locale(CollationCacheContext,
+ CollationCacheOwner,
+ collid);
last_collation_cache_oid = collid;
last_collation_cache_locale = cache_entry->locale;
--
2.34.1
From 2f51247615a36dc257b700c2832f3d4aa32fce64 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Wed, 18 Sep 2024 17:49:57 -0700
Subject: [PATCH v4 5/7] invalidation
---
src/backend/utils/adt/pg_locale.c | 41 +++++++++++++++++++++++++------
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 9d1d71f1561..c89ac3b9e01 100644
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -63,6 +63,7 @@
#include "utils/builtins.h"
#include "utils/formatting.h"
#include "utils/guc_hooks.h"
+#include "utils/inval.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
#include "utils/pg_locale.h"
@@ -1695,6 +1696,34 @@ create_pg_locale(MemoryContext context, ResourceOwner owner, Oid collid)
return result;
}
+static void
+CollationCacheInvalidate(Datum arg, int cacheid, uint32 hashvalue)
+{
+ last_collation_cache_oid = InvalidOid;
+
+ if (CollationCache == NULL)
+ return;
+
+ ResourceOwnerRelease(CollationCacheOwner,
+ RESOURCE_RELEASE_BEFORE_LOCKS,
+ false, true);
+ ResourceOwnerRelease(CollationCacheOwner,
+ RESOURCE_RELEASE_LOCKS,
+ false, true);
+ ResourceOwnerRelease(CollationCacheOwner,
+ RESOURCE_RELEASE_AFTER_LOCKS,
+ false, true);
+ ResourceOwnerDelete(CollationCacheOwner);
+ CollationCacheOwner = ResourceOwnerCreate(NULL, "collation cache");
+
+ MemoryContextReset(CollationCacheContext);
+
+ /* free all memory and reset hash table */
+ CollationCache = collation_cache_create(CollationCacheContext,
+ 16, NULL);
+}
+
+
/*
* Create or retrieve a pg_locale_t for the given collation OID. Results are
* cached for the lifetime of the backend.
@@ -1714,14 +1743,7 @@ pg_newlocale_from_collation(Oid collid)
if (last_collation_cache_oid == collid)
return last_collation_cache_locale;
- /*
- * Cache mechanism for collation information.
- *
- * Note that we currently lack any way to flush the cache. Since we don't
- * support ALTER COLLATION, this is OK. The worst case is that someone
- * drops a collation, and a useless cache entry hangs around in existing
- * backends.
- */
+ /* cache mechanism for collation information */
if (CollationCache == NULL)
{
CollationCacheOwner = ResourceOwnerCreate(NULL, "collation cache");
@@ -1730,6 +1752,9 @@ pg_newlocale_from_collation(Oid collid)
ALLOCSET_DEFAULT_SIZES);
CollationCache = collation_cache_create(CollationCacheContext,
16, NULL);
+ CacheRegisterSyscacheCallback(COLLOID,
+ CollationCacheInvalidate,
+ (Datum) 0);
}
cache_entry = collation_cache_insert(CollationCache, collid, &found);
--
2.34.1
