From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@postgreSQL.org
Subject: Bizarre behavior of literal-substring parsing in formatting.c
Date: Fri, 17 Nov 2017 15:11:26 -0500
Message-ID: <3626.1510949486@sss.pgh.pa.us> (raw)
I started to look at handling multibyte data more sanely in formatting.c,
and soon noticed that the treatment of literal substrings in
parse_format() seemed overly complicated and underly correct, as well
as inadequately documented. In particular, it can't make up its mind
whether backslash is a quoting character or not. These cases make it
look like it is:
regression=# select to_char('100'::numeric, 'f"\ool"999');
to_char
----------
fool 100
(1 row)
regression=# select to_char('100'::numeric, 'f"\\ool"999');
to_char
-----------
f\ool 100
(1 row)
regression=# select to_char('100'::numeric, 'f"ool\"999');
to_char
----------
fool"999
(1 row)
(Since the second " is quoted, we never escape the literal substring,
and the format-code 999 becomes just literal data.) But try this:
regression=# select to_char('100'::numeric, 'f"ool\\"999');
to_char
-----------
fool\"999
(1 row)
The first backslash has quoted the second one, and then reached out
and quoted the quote too. That hardly seems sane.
Outside double quotes, it seems that backslash is only special when it
immediately precedes a double quote. That's not enormously consistent
with other string parsers in our code, but it's at least self-consistent,
and given the lack of complaints it's probably best not to change that
behavior.
Accordingly I offer the attached rewrite. Of the new regression
test cases, the only one that changes behavior from before is the
last one, which is the case I just showed.
If no objections, I'll push this shortly. I think putting it in
HEAD is enough, given the lack of field complaints.
regards, tom lane
Attachments:fix-formatting.c-literal-parsing.patch (text/x-diff, 7.8K) downloaddiff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgmlindex 35a845c..698daf6 100644
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*************** SELECT regexp_match('abc01234xyz', '(?:(
*** 6196,6201 ****
--- 6196,6206 ----
If you want to have a double quote in the output you must
precede it with a backslash, for example <literal>'\"YYYY
Month\"'</literal>. <!-- "" font-lock sanity :-) -->
+ 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).
</para>
</listitem>
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.cindex 5afc293..cb0dbf7 100644
*** a/src/backend/utils/adt/formatting.c
--- b/src/backend/utils/adt/formatting.c
*************** static void
*** 1227,1237 ****
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");
--- 1227,1233 ----
*************** parse_format(FormatNode *node, const cha
*** 1241,1252 ****
while (*str)
{
! suffix = 0;
/*
* Prefix
*/
! if (ver == DCH_TYPE && (s = suff_search(str, suf, SUFFTYPE_PREFIX)) != NULL)
{
suffix |= s->id;
if (s->len)
--- 1237,1250 ----
while (*str)
{
! int suffix = 0;
! const KeySuffix *s;
/*
* Prefix
*/
! if (ver == DCH_TYPE &&
! (s = suff_search(str, suf, SUFFTYPE_PREFIX)) != NULL)
{
suffix |= s->id;
if (s->len)
*************** parse_format(FormatNode *node, const cha
*** 1259,1266 ****
if (*str && (n->key = index_seq_search(str, kw, index)) != NULL)
{
n->type = NODE_TYPE_ACTION;
! n->suffix = 0;
! node_set = 1;
if (n->key->len)
str += n->key->len;
--- 1257,1263 ----
if (*str && (n->key = index_seq_search(str, kw, index)) != NULL)
{
n->type = NODE_TYPE_ACTION;
! n->suffix = suffix;
if (n->key->len)
str += n->key->len;
*************** parse_format(FormatNode *node, const cha
*** 1273,1343 ****
/*
* Postfix
*/
! if (ver == DCH_TYPE && *str && (s = suff_search(str, suf, SUFFTYPE_POSTFIX)) != NULL)
{
! suffix |= s->id;
if (s->len)
str += s->len;
}
}
else if (*str)
{
/*
! * Special characters '\' and '"'
*/
! if (*str == '"' && last != '\\')
{
- int x = 0;-
while (*(++str))
{
! if (*str == '"' && x != '\\')
{
str++;
break;
}
! else if (*str == '\\' && x != '\\')
! {
! x = '\\';
! continue;
! }
n->type = NODE_TYPE_CHAR;
n->character = *str;
n->key = NULL;
n->suffix = 0;
! ++n;
! x = *str;
}
- node_set = 0;- suffix = 0;- last = 0;- }- else if (*str && *str == '\\' && last != '\\' && *(str + 1) == '"')- {- last = *str;- str++;
}
! else if (*str)
{
n->type = NODE_TYPE_CHAR;
n->character = *str;
n->key = NULL;
! node_set = 1;
! last = 0;
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;
--- 1270,1325 ----
/*
* Postfix
*/
! if (ver == DCH_TYPE && *str &&
! (s = suff_search(str, suf, SUFFTYPE_POSTFIX)) != NULL)
{
! n->suffix |= s->id;
if (s->len)
str += s->len;
}
+ + n++;
}
else if (*str)
{
/*
! * Process double-quoted literal string, if any
*/
! if (*str == '"')
{
while (*(++str))
{
! if (*str == '"')
{
str++;
break;
}
! /* 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++;
}
}
! 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;
! n->suffix = 0;
! n++;
str++;
}
}
}
n->type = NODE_TYPE_END;
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.outindex a96bfc0..17985e8 100644
*** a/src/test/regress/expected/numeric.out
--- b/src/test/regress/expected/numeric.out
*************** SELECT '' AS to_char_26, to_char('100'::
*** 1217,1222 ****
--- 1217,1283 ----
| 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.sqlindex 321c7bd..d77504e 100644
*** a/src/test/regress/sql/numeric.sql
--- b/src/test/regress/sql/numeric.sql
*************** SELECT '' AS to_char_24, to_char('100'::
*** 786,791 ****
--- 786,803 ----
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';
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us
Subject: Re: Bizarre behavior of literal-substring parsing in formatting.c
In-Reply-To: <3626.1510949486@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox