From 63ca86318dc3d6a768eed78efbc6ca014a0622a8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 18 Nov 2017 12:16:37 -0500 Subject: [PATCH] Fix quoted-substring handling in format parsing for to_char/to_number/etc. This code evidently intended to treat backslash as an escape character within double-quoted substrings, but it was sufficiently confused that cases like ..."foo\\"... did not work right: the second backslash managed to quote the double-quote after it, despite being quoted itself. Rewrite to get that right, while preserving the existing behavior outside double-quoted substrings, which is that backslash isn't special except in the combination \". Comparing to Oracle, it seems that their version of to_char() for timestamps allows literal alphanumerics only within double quotes, while non-alphanumerics are allowed outside quotes; backslashes aren't special anywhere; there is no way at all to emit a literal double quote. (Bizarrely, their to_char() for numbers is different; it doesn't allow literal text at all AFAICT.) The fact that they don't treat backslash as special justifies our existing behavior for backslash outside double quotes. I considered making backslash inside double quotes act the same way (ie, special only if before "), which in a green field would be a more consistent behavior. But that would likely break more existing SQL code than what this patch does. Add some test cases illustrating this behavior. (Only the last new case actually changes behavior in this commit.) Little of this behavior was documented, either, so fix that. Discussion: https://postgr.es/m/3626.1510949486@sss.pgh.pa.us --- doc/src/sgml/func.sgml | 5 ++ src/backend/utils/adt/formatting.c | 70 ++++++++++----------------- src/test/regress/expected/numeric.out | 61 +++++++++++++++++++++++ src/test/regress/sql/numeric.sql | 12 +++++ 4 files changed, 104 insertions(+), 44 deletions(-) diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 35a845c400..698daf69ea 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -6196,6 +6196,11 @@ SELECT regexp_match('abc01234xyz', '(?:(.*?)(\d+)(.*)){1,1}'); If you want to have a double quote in the output you must precede it with a backslash, for example '\"YYYY Month\"'. + Backslashes are not otherwise special outside of double-quoted + strings. Within a double-quoted string, a backslash causes the + next character to be taken literally, whatever it is (but this + has no special effect unless the next character is a double quote + or another backslash). diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 5afc293a5a..cb0dbf748e 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -1227,11 +1227,7 @@ static void parse_format(FormatNode *node, const char *str, const KeyWord *kw, const KeySuffix *suf, const int *index, int ver, NUMDesc *Num) { - const KeySuffix *s; FormatNode *n; - int node_set = 0, - suffix, - last = 0; #ifdef DEBUG_TO_FROM_CHAR elog(DEBUG_elog_output, "to_char/number(): run parser"); @@ -1241,12 +1237,14 @@ parse_format(FormatNode *node, const char *str, const KeyWord *kw, while (*str) { - suffix = 0; + int suffix = 0; + const KeySuffix *s; /* * Prefix */ - if (ver == DCH_TYPE && (s = suff_search(str, suf, SUFFTYPE_PREFIX)) != NULL) + if (ver == DCH_TYPE && + (s = suff_search(str, suf, SUFFTYPE_PREFIX)) != NULL) { suffix |= s->id; if (s->len) @@ -1259,8 +1257,7 @@ parse_format(FormatNode *node, const char *str, const KeyWord *kw, if (*str && (n->key = index_seq_search(str, kw, index)) != NULL) { n->type = NODE_TYPE_ACTION; - n->suffix = 0; - node_set = 1; + n->suffix = suffix; if (n->key->len) str += n->key->len; @@ -1273,71 +1270,56 @@ parse_format(FormatNode *node, const char *str, const KeyWord *kw, /* * Postfix */ - if (ver == DCH_TYPE && *str && (s = suff_search(str, suf, SUFFTYPE_POSTFIX)) != NULL) + if (ver == DCH_TYPE && *str && + (s = suff_search(str, suf, SUFFTYPE_POSTFIX)) != NULL) { - suffix |= s->id; + n->suffix |= s->id; if (s->len) str += s->len; } + + n++; } else if (*str) { /* - * Special characters '\' and '"' + * Process double-quoted literal string, if any */ - if (*str == '"' && last != '\\') + if (*str == '"') { - int x = 0; - while (*(++str)) { - if (*str == '"' && x != '\\') + if (*str == '"') { str++; break; } - else if (*str == '\\' && x != '\\') - { - x = '\\'; - continue; - } + /* backslash quotes the next character, if any */ + if (*str == '\\' && *(str + 1)) + str++; n->type = NODE_TYPE_CHAR; n->character = *str; n->key = NULL; n->suffix = 0; - ++n; - x = *str; + n++; } - node_set = 0; - suffix = 0; - last = 0; } - else if (*str && *str == '\\' && last != '\\' && *(str + 1) == '"') - { - last = *str; - str++; - } - else if (*str) + else { + /* + * Outside double-quoted strings, backslash is only special if + * it immediately precedes a double quote. + */ + if (*str == '\\' && *(str + 1) == '"') + str++; n->type = NODE_TYPE_CHAR; n->character = *str; n->key = NULL; - node_set = 1; - last = 0; + n->suffix = 0; + n++; str++; } } - - /* end */ - if (node_set) - { - if (n->type == NODE_TYPE_ACTION) - n->suffix = suffix; - ++n; - - n->suffix = 0; - node_set = 0; - } } n->type = NODE_TYPE_END; diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out index a96bfc0eb0..17985e8540 100644 --- a/src/test/regress/expected/numeric.out +++ b/src/test/regress/expected/numeric.out @@ -1217,6 +1217,67 @@ SELECT '' AS to_char_26, to_char('100'::numeric, 'FM999'); | 100 (1 row) +-- Check parsing of literal text in a format string +SELECT '' AS to_char_27, to_char('100'::numeric, 'foo999'); + to_char_27 | to_char +------------+--------- + | foo 100 +(1 row) + +SELECT '' AS to_char_28, to_char('100'::numeric, 'f\oo999'); + to_char_28 | to_char +------------+---------- + | f\oo 100 +(1 row) + +SELECT '' AS to_char_29, to_char('100'::numeric, 'f\\oo999'); + to_char_29 | to_char +------------+----------- + | f\\oo 100 +(1 row) + +SELECT '' AS to_char_30, to_char('100'::numeric, 'f\"oo999'); + to_char_30 | to_char +------------+---------- + | f"oo 100 +(1 row) + +SELECT '' AS to_char_31, to_char('100'::numeric, 'f\\"oo999'); + to_char_31 | to_char +------------+----------- + | f\"oo 100 +(1 row) + +SELECT '' AS to_char_32, to_char('100'::numeric, 'f"ool"999'); + to_char_32 | to_char +------------+---------- + | fool 100 +(1 row) + +SELECT '' AS to_char_33, to_char('100'::numeric, 'f"\ool"999'); + to_char_33 | to_char +------------+---------- + | fool 100 +(1 row) + +SELECT '' AS to_char_34, to_char('100'::numeric, 'f"\\ool"999'); + to_char_34 | to_char +------------+----------- + | f\ool 100 +(1 row) + +SELECT '' AS to_char_35, to_char('100'::numeric, 'f"ool\"999'); + to_char_35 | to_char +------------+---------- + | fool"999 +(1 row) + +SELECT '' AS to_char_36, to_char('100'::numeric, 'f"ool\\"999'); + to_char_36 | to_char +------------+----------- + | fool\ 100 +(1 row) + -- TO_NUMBER() -- SET lc_numeric = 'C'; diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql index 321c7bdf7c..d77504e624 100644 --- a/src/test/regress/sql/numeric.sql +++ b/src/test/regress/sql/numeric.sql @@ -786,6 +786,18 @@ SELECT '' AS to_char_24, to_char('100'::numeric, 'FM999.9'); SELECT '' AS to_char_25, to_char('100'::numeric, 'FM999.'); SELECT '' AS to_char_26, to_char('100'::numeric, 'FM999'); +-- Check parsing of literal text in a format string +SELECT '' AS to_char_27, to_char('100'::numeric, 'foo999'); +SELECT '' AS to_char_28, to_char('100'::numeric, 'f\oo999'); +SELECT '' AS to_char_29, to_char('100'::numeric, 'f\\oo999'); +SELECT '' AS to_char_30, to_char('100'::numeric, 'f\"oo999'); +SELECT '' AS to_char_31, to_char('100'::numeric, 'f\\"oo999'); +SELECT '' AS to_char_32, to_char('100'::numeric, 'f"ool"999'); +SELECT '' AS to_char_33, to_char('100'::numeric, 'f"\ool"999'); +SELECT '' AS to_char_34, to_char('100'::numeric, 'f"\\ool"999'); +SELECT '' AS to_char_35, to_char('100'::numeric, 'f"ool\"999'); +SELECT '' AS to_char_36, to_char('100'::numeric, 'f"ool\\"999'); + -- TO_NUMBER() -- SET lc_numeric = 'C';