Improve to_date/to_number/to_timestamp behavior with multibyte characters.

The documentation says that these functions skip one input character
per literal (non-pattern) format character.  Actually, though, they
skipped one input *byte* per literal *byte*, which could be hugely
confusing if either data or format contained multibyte characters.

To fix, adjust the FormatNode representation and parse_format() so
that multibyte format characters are stored as one FormatNode not
several, and adjust the data-skipping bits to advance by pg_mblen()
not necessarily one byte.  There's no user-visible behavior change
on the to_char() side, although the internal representation changes.

Commit e87d4965b had already fixed most places where we skip characters
on the basis of non-literal format patterns to advance by characters
not bytes, but this gets one more place, the SKIP_THth macro.  I think
everything in formatting.c gets that right now.

It'd be nice to have some regression test cases covering this behavior;
but of course there's no way to do so in an encoding-agnostic way, and
many of the interesting aspects would also require unportable locale
selections.  So I've not bothered here.

Discussion: https://postgr.es/m/28186.1510957703@sss.pgh.pa.us

diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index cb0dbf748e501f0ba002efe37d48e9e9c4df12a8..ec97de0ad27ee9c29731f9138c965fb07c1f103a 100644 (file)
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -151,8 +151,6 @@ typedef enum
    FROM_CHAR_DATE_ISOWEEK      /* ISO 8601 week date */
 } FromCharDateMode;
 
-typedef struct FormatNode FormatNode;
-
 typedef struct
 {
    const char *name;
@@ -162,13 +160,13 @@ typedef struct
    FromCharDateMode date_mode;
 } KeyWord;
 
-struct FormatNode
+typedef struct
 {
-   int         type;           /* node type            */
-   const KeyWord *key;         /* if node type is KEYWORD  */
-   char        character;      /* if node type is CHAR     */
-   int         suffix;         /* keyword suffix       */
-};
+   int         type;           /* NODE_TYPE_XXX, see below */
+   const KeyWord *key;         /* if type is ACTION */
+   char        character[MAX_MULTIBYTE_CHAR_LEN + 1];  /* if type is CHAR */
+   int         suffix;         /* keyword prefix/suffix code, if any */
+} FormatNode;
 
 #define NODE_TYPE_END      1
 #define NODE_TYPE_ACTION   2
@@ -1282,12 +1280,15 @@ parse_format(FormatNode *node, const char *str, const KeyWord *kw,
        }
        else if (*str)
        {
+           int         chlen;
+
            /*
             * Process double-quoted literal string, if any
             */
            if (*str == '"')
            {
-               while (*(++str))
+               str++;
+               while (*str)
                {
                    if (*str == '"')
                    {
@@ -1297,11 +1298,14 @@ parse_format(FormatNode *node, const char *str, const KeyWord *kw,
                    /* backslash quotes the next character, if any */
                    if (*str == '\\' && *(str + 1))
                        str++;
+                   chlen = pg_mblen(str);
                    n->type = NODE_TYPE_CHAR;
-                   n->character = *str;
+                   memcpy(n->character, str, chlen);
+                   n->character[chlen] = '\0';
                    n->key = NULL;
                    n->suffix = 0;
                    n++;
+                   str += chlen;
                }
            }
            else
@@ -1312,12 +1316,14 @@ parse_format(FormatNode *node, const char *str, const KeyWord *kw,
                 */
                if (*str == '\\' && *(str + 1) == '"')
                    str++;
+               chlen = pg_mblen(str);
                n->type = NODE_TYPE_CHAR;
-               n->character = *str;
+               memcpy(n->character, str, chlen);
+               n->character[chlen] = '\0';
                n->key = NULL;
                n->suffix = 0;
                n++;
-               str++;
+               str += chlen;
            }
        }
    }
@@ -1349,7 +1355,8 @@ dump_node(FormatNode *node, int max)
            elog(DEBUG_elog_output, "%d:\t NODE_TYPE_ACTION '%s'\t(%s,%s)",
                 a, n->key->name, DUMP_THth(n->suffix), DUMP_FM(n->suffix));
        else if (n->type == NODE_TYPE_CHAR)
-           elog(DEBUG_elog_output, "%d:\t NODE_TYPE_CHAR '%c'", a, n->character);
+           elog(DEBUG_elog_output, "%d:\t NODE_TYPE_CHAR '%s'",
+                a, n->character);
        else if (n->type == NODE_TYPE_END)
        {
            elog(DEBUG_elog_output, "%d:\t NODE_TYPE_END", a);
@@ -2008,8 +2015,8 @@ asc_toupper_z(const char *buff)
    do { \
        if (S_THth(_suf)) \
        { \
-           if (*(ptr)) (ptr)++; \
-           if (*(ptr)) (ptr)++; \
+           if (*(ptr)) (ptr) += pg_mblen(ptr); \
+           if (*(ptr)) (ptr) += pg_mblen(ptr); \
        } \
    } while (0)
 
@@ -2076,7 +2083,8 @@ is_next_separator(FormatNode *n)
 
        return true;
    }
-   else if (isdigit((unsigned char) n->character))
+   else if (n->character[1] == '\0' &&
+            isdigit((unsigned char) n->character[0]))
        return false;
 
    return true;                /* some non-digit input (separator) */
@@ -2405,8 +2413,8 @@ DCH_to_char(FormatNode *node, bool is_interval, TmToChar *in, char *out, Oid col
    {
        if (n->type != NODE_TYPE_ACTION)
        {
-           *s = n->character;
-           s++;
+           strcpy(s, n->character);
+           s += strlen(s);
            continue;
        }
 
@@ -2974,7 +2982,7 @@ DCH_from_char(FormatNode *node, char *in, TmFromChar *out)
             * we don't insist that the consumed character match the format's
             * character.
             */
-           s++;
+           s += pg_mblen(s);
            continue;
        }
 
@@ -4217,7 +4225,7 @@ get_last_relevant_decnum(char *num)
 /*
  * These macros are used in NUM_processor() and its subsidiary routines.
  * OVERLOAD_TEST: true if we've reached end of input string
- * AMOUNT_TEST(s): true if at least s characters remain in string
+ * AMOUNT_TEST(s): true if at least s bytes remain in string
  */
 #define OVERLOAD_TEST  (Np->inout_p >= Np->inout + input_len)
 #define AMOUNT_TEST(s) (Np->inout_p <= Np->inout + (input_len - (s)))
@@ -4821,9 +4829,9 @@ NUM_processor(FormatNode *node, NUMDesc *Num, char *inout,
        if (!Np->is_to_char)
        {
            /*
-            * Check at least one character remains to be scanned.  (In
-            * actions below, must use AMOUNT_TEST if we want to read more
-            * characters than that.)
+            * Check at least one byte remains to be scanned.  (In actions
+            * below, must use AMOUNT_TEST if we want to read more bytes than
+            * that.)
             */
            if (OVERLOAD_TEST)
                break;
@@ -5081,12 +5089,18 @@ NUM_processor(FormatNode *node, NUMDesc *Num, char *inout,
             * In TO_CHAR, non-pattern characters in the format are copied to
             * the output.  In TO_NUMBER, we skip one input character for each
             * non-pattern format character, whether or not it matches the
-            * format character.  (Currently, that's actually implemented as
-            * skipping one input byte per non-pattern format byte, which is
-            * wrong...)
+            * format character.
             */
            if (Np->is_to_char)
-               *Np->inout_p = n->character;
+           {
+               strcpy(Np->inout_p, n->character);
+               Np->inout_p += strlen(Np->inout_p);
+           }
+           else
+           {
+               Np->inout_p += pg_mblen(Np->inout_p);
+           }
+           continue;
        }
        Np->inout_p++;
    }