selfuncs.c: use pg_strxfrm() instead of strxfrm().

pg_strxfrm() takes a pg_locale_t, so it works properly with all
providers. This improves estimates for ICU when performing linear
interpolation within a histogram bin.

Previously, convert_string_datum() always used strxfrm() and relied on
setlocale(). That did not produce good estimates for non-default or
non-libc collations.

Discussion: https://postgr.es/m/89475ee5487d795124f4e25118ea8f1853edb8cb[email protected]

diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 4e076904585f1fe92464c3ccbec7c43314a34778..cd3661e7279d4056a7251614e473a79974f5408c 100644 (file)
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -2124,14 +2124,7 @@ pg_strxfrm_libc(char *dest, const char *src, size_t destsize,
                pg_locale_t locale)
 {
    Assert(locale->provider == COLLPROVIDER_LIBC);
-
-#ifdef TRUST_STRXFRM
    return strxfrm_l(dest, src, destsize, locale->info.lt);
-#else
-   /* shouldn't happen */
-   PGLOCALE_SUPPORT_ERROR(locale->provider);
-   return 0;                   /* keep compiler quiet */
-#endif
 }
 
 static size_t
@@ -2340,6 +2333,10 @@ pg_strxfrm_enabled(pg_locale_t locale)
  * The provided 'src' must be nul-terminated. If 'destsize' is zero, 'dest'
  * may be NULL.
  *
+ * Not all providers support pg_strxfrm() safely. The caller should check
+ * pg_strxfrm_enabled() first, otherwise this function may return wrong
+ * results or an error.
+ *
  * Returns the number of bytes needed (or more) to store the transformed
  * string, excluding the terminating nul byte. If the value returned is
  * 'destsize' or greater, the resulting contents of 'dest' are undefined.
@@ -2372,6 +2369,10 @@ pg_strxfrm(char *dest, const char *src, size_t destsize, pg_locale_t locale)
  * 'src' does not need to be nul-terminated. If 'destsize' is zero, 'dest' may
  * be NULL.
  *
+ * Not all providers support pg_strnxfrm() safely. The caller should check
+ * pg_strxfrm_enabled() first, otherwise this function may return wrong
+ * results or an error.
+ *
  * Returns the number of bytes needed (or more) to store the transformed
  * string, excluding the terminating nul byte. If the value returned is
  * 'destsize' or greater, the resulting contents of 'dest' are undefined.
@@ -2426,6 +2427,10 @@ pg_strxfrm_prefix_enabled(pg_locale_t locale)
  *
  * The provided 'src' must be nul-terminated.
  *
+ * Not all providers support pg_strxfrm_prefix() safely. The caller should
+ * check pg_strxfrm_prefix_enabled() first, otherwise this function may return
+ * wrong results or an error.
+ *
  * If destsize is not large enough to hold the resulting byte sequence, stores
  * only the first destsize bytes in 'dest'. Returns the number of bytes
  * actually copied to 'dest'.
@@ -2455,6 +2460,10 @@ pg_strxfrm_prefix(char *dest, const char *src, size_t destsize,
  *
  * The provided 'src' must be nul-terminated.
  *
+ * Not all providers support pg_strnxfrm_prefix() safely. The caller should
+ * check pg_strxfrm_prefix_enabled() first, otherwise this function may return
+ * wrong results or an error.
+ *
  * If destsize is not large enough to hold the resulting byte sequence, stores
  * only the first destsize bytes in 'dest'. Returns the number of bytes
  * actually copied to 'dest'.

diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 877a62a62ec7acfed013c49bf489942b7c17c7ed..bf42393bec6d10595996fe3eb835dc3f9f221951 100644 (file)
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -4639,7 +4639,7 @@ convert_one_string_to_scalar(char *value, int rangelo, int rangehi)
  * On failure (e.g., unsupported typid), set *failure to true;
  * otherwise, that variable is not changed.  (We'll return NULL on failure.)
  *
- * When using a non-C locale, we must pass the string through strxfrm()
+ * When using a non-C locale, we must pass the string through pg_strxfrm()
  * before continuing, so as to generate correct locale-specific results.
  */
 static char *
@@ -4673,20 +4673,25 @@ convert_string_datum(Datum value, Oid typid, Oid collid, bool *failure)
 
    if (!lc_collate_is_c(collid))
    {
+       pg_locale_t mylocale = pg_newlocale_from_collation(collid);
        char       *xfrmstr;
        size_t      xfrmlen;
        size_t      xfrmlen2 PG_USED_FOR_ASSERTS_ONLY;
 
        /*
         * XXX: We could guess at a suitable output buffer size and only call
-        * strxfrm twice if our guess is too small.
+        * pg_strxfrm() twice if our guess is too small.
         *
         * XXX: strxfrm doesn't support UTF-8 encoding on Win32, it can return
         * bogus data or set an error. This is not really a problem unless it
         * crashes since it will only give an estimation error and nothing
         * fatal.
+        *
+        * XXX: we do not check pg_strxfrm_enabled(). On some platforms and in
+        * some cases, libc strxfrm() may return the wrong results, but that
+        * will only lead to an estimation error.
         */
-       xfrmlen = strxfrm(NULL, val, 0);
+       xfrmlen = pg_strxfrm(NULL, val, 0, mylocale);
 #ifdef WIN32
 
        /*
@@ -4698,7 +4703,7 @@ convert_string_datum(Datum value, Oid typid, Oid collid, bool *failure)
            return val;
 #endif
        xfrmstr = (char *) palloc(xfrmlen + 1);
-       xfrmlen2 = strxfrm(xfrmstr, val, xfrmlen + 1);
+       xfrmlen2 = pg_strxfrm(xfrmstr, val, xfrmlen + 1, mylocale);
 
        /*
         * Some systems (e.g., glibc) can return a smaller value from the