From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Gierth <andrew@tao11.riddles.org.uk>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: SQL-spec incompatibilities in similar_escape() and related stuff
Date: Thu, 23 May 2019 18:10:24 -0400
Message-ID: <32617.1558649424@sss.pgh.pa.us> (raw)
In-Reply-To: <21793.1557850923@sss.pgh.pa.us>
References: <14047.1557708214@sss.pgh.pa.us>
<8736lihni6.fsf@news-spur.riddles.org.uk>
<21793.1557850923@sss.pgh.pa.us>
I wrote:
> I propose therefore that we leave similar_escape in place with its
> current behavior, as a compatibility measure for cases like this.
> Intead, invent two new strict functions, say
> similar_to_escape(pattern)
> similar_to_escape(pattern, escape)
> and change the parser and the implementation of SUBSTRING() to
> rely on these going forward.
> The net effect will be to make explicit "ESCAPE NULL" spec-compliant,
> and to get rid of the performance problem from inlining failure for
> substring(). All else is just doc clarifications.
Here's a proposed patch for that. I think it's a bit too late to be
messing with this kind of thing for v12, so I'll add this to the
upcoming CF.
regards, tom lane
Attachments:
[text/x-diff] make-similar-to-escape-null-handling-sane.patch (12.3K, ../32617.1558649424@sss.pgh.pa.us/2-make-similar-to-escape-null-handling-sane.patch)
download | inline diff:diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgmlindex a79e7c0..7d4472a 100644--- a/doc/src/sgml/func.sgml+++ b/doc/src/sgml/func.sgml@@ -4146,6 +4146,14 @@ cast(-44 as bit(12)) <lineannotation>111111010100</lineannotation>
</para>
<para>
+ According to the SQL standard, omitting <literal>ESCAPE</literal>+ means there is no escape character (rather than defaulting to a+ backslash), and a zero-length <literal>ESCAPE</literal> value is+ disallowed. <productname>PostgreSQL</productname>'s behavior in+ this regard is therefore slightly nonstandard.+ </para>++ <para>
The key word <token>ILIKE</token> can be used instead of
<token>LIKE</token> to make the match case-insensitive according
to the active locale. This is not in the <acronym>SQL</acronym> standard but is a
@@ -4280,9 +4288,27 @@ cast(-44 as bit(12)) <lineannotation>111111010100</lineannotation>
</para>
<para>
- As with <function>LIKE</function>, a backslash disables the special meaning- of any of these metacharacters; or a different escape character can- be specified with <literal>ESCAPE</literal>.+ As with <function>LIKE</function>, a backslash disables the special+ meaning of any of these metacharacters. A different escape character+ can be specified with <literal>ESCAPE</literal>, or the escape+ capability can be disabled by writing <literal>ESCAPE ''</literal>.+ </para>++ <para>+ According to the SQL standard, omitting <literal>ESCAPE</literal>+ means there is no escape character (rather than defaulting to a+ backslash), and a zero-length <literal>ESCAPE</literal> value is+ disallowed. <productname>PostgreSQL</productname>'s behavior in+ this regard is therefore slightly nonstandard.+ </para>++ <para>+ Another nonstandard extension is that following the escape character+ with a letter or digit provides access to the same escape sequences+ defined for POSIX regular expressions, below (see+ <xref linkend="posix-character-entry-escapes-table"/>,+ <xref linkend="posix-class-shorthand-escapes-table"/>, and+ <xref linkend="posix-constraint-escapes-table"/>).
</para>
<para>
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.yindex 8311b1d..6462e13 100644--- a/src/backend/parser/gram.y+++ b/src/backend/parser/gram.y@@ -13073,15 +13073,15 @@ a_expr: c_expr { $$ = $1; }
| a_expr SIMILAR TO a_expr %prec SIMILAR
{
- FuncCall *n = makeFuncCall(SystemFuncName("similar_escape"),- list_make2($4, makeNullAConst(-1)),+ FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"),+ list_make1($4),
@2);
$$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "~",
$1, (Node *) n, @2);
}
| a_expr SIMILAR TO a_expr ESCAPE a_expr %prec SIMILAR
{
- FuncCall *n = makeFuncCall(SystemFuncName("similar_escape"),+ FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"),
list_make2($4, $6),
@2);
$$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "~",
@@ -13089,15 +13089,15 @@ a_expr: c_expr { $$ = $1; }
}
| a_expr NOT_LA SIMILAR TO a_expr %prec NOT_LA
{
- FuncCall *n = makeFuncCall(SystemFuncName("similar_escape"),- list_make2($5, makeNullAConst(-1)),+ FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"),+ list_make1($5),
@2);
$$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "!~",
$1, (Node *) n, @2);
}
| a_expr NOT_LA SIMILAR TO a_expr ESCAPE a_expr %prec NOT_LA
{
- FuncCall *n = makeFuncCall(SystemFuncName("similar_escape"),+ FuncCall *n = makeFuncCall(SystemFuncName("similar_to_escape"),
list_make2($5, $7),
@2);
$$ = (Node *) makeSimpleA_Expr(AEXPR_SIMILAR, "!~",
@@ -14323,9 +14323,9 @@ subquery_Op:
| NOT_LA ILIKE
{ $$ = list_make1(makeString("!~~*")); }
/* cannot put SIMILAR TO here, because SIMILAR TO is a hack.
- * the regular expression is preprocessed by a function (similar_escape),+ * the regular expression is preprocessed by a function (similar_to_escape),
* and the ~ operator for posix regular expressions is used.
- * x SIMILAR TO y -> x ~ similar_escape(y)+ * x SIMILAR TO y -> x ~ similar_to_escape(y)
* this transformation is made on the fly by the parser upwards.
* however the SubLink structure which handles any/some/all stuff
* is not ready for such a thing.
diff --git a/src/backend/utils/adt/regexp.c b/src/backend/utils/adt/regexp.cindex 90a9197..3d38aef 100644--- a/src/backend/utils/adt/regexp.c+++ b/src/backend/utils/adt/regexp.c@@ -654,15 +654,18 @@ textregexreplace(PG_FUNCTION_ARGS)
}
/*
- * similar_escape()- * Convert a SQL:2008 regexp pattern to POSIX style, so it can be used by- * our regexp engine.+ * similar_to_escape(), similar_escape()+ *+ * Convert a SQL "SIMILAR TO" regexp pattern to POSIX style, so it can be+ * used by our regexp engine.+ *+ * similar_escape_internal() is the common workhorse for three SQL-exposed+ * functions. esc_text can be passed as NULL to select the default escape+ * (which is '\'), or as an empty string to select no escape character.
*/
-Datum-similar_escape(PG_FUNCTION_ARGS)+static text *+similar_escape_internal(text *pat_text, text *esc_text)
{
- text *pat_text;- text *esc_text;
text *result;
char *p,
*e,
@@ -673,13 +676,9 @@ similar_escape(PG_FUNCTION_ARGS)
bool incharclass = false;
int nquotes = 0;
- /* This function is not strict, so must test explicitly */- if (PG_ARGISNULL(0))- PG_RETURN_NULL();- pat_text = PG_GETARG_TEXT_PP(0);
p = VARDATA_ANY(pat_text);
plen = VARSIZE_ANY_EXHDR(pat_text);
- if (PG_ARGISNULL(1))+ if (esc_text == NULL)
{
/* No ESCAPE clause provided; default to backslash as escape */
e = "\\";
@@ -687,12 +686,11 @@ similar_escape(PG_FUNCTION_ARGS)
}
else
{
- esc_text = PG_GETARG_TEXT_PP(1);
e = VARDATA_ANY(esc_text);
elen = VARSIZE_ANY_EXHDR(esc_text);
if (elen == 0)
e = NULL; /* no escape character */
- else+ else if (elen > 1)
{
int escape_mblen = pg_mbstrlen_with_len(e, elen);
@@ -898,6 +896,65 @@ similar_escape(PG_FUNCTION_ARGS)
SET_VARSIZE(result, r - ((char *) result));
+ return result;+}++/*+ * similar_to_escape(pattern, escape)+ */+Datum+similar_to_escape_2(PG_FUNCTION_ARGS)+{+ text *pat_text = PG_GETARG_TEXT_PP(0);+ text *esc_text = PG_GETARG_TEXT_PP(1);+ text *result;++ result = similar_escape_internal(pat_text, esc_text);++ PG_RETURN_TEXT_P(result);+}++/*+ * similar_to_escape(pattern)+ * Inserts a default escape character.+ */+Datum+similar_to_escape_1(PG_FUNCTION_ARGS)+{+ text *pat_text = PG_GETARG_TEXT_PP(0);+ text *result;++ result = similar_escape_internal(pat_text, NULL);++ PG_RETURN_TEXT_P(result);+}++/*+ * similar_escape(pattern, escape)+ *+ * Legacy function for compatibility with views stored using the+ * pre-v13 expansion of SIMILAR TO. Unlike the above functions, this+ * is non-strict, which leads to not-per-spec handling of "ESCAPE NULL".+ */+Datum+similar_escape(PG_FUNCTION_ARGS)+{+ text *pat_text;+ text *esc_text;+ text *result;++ /* This function is not strict, so must test explicitly */+ if (PG_ARGISNULL(0))+ PG_RETURN_NULL();+ pat_text = PG_GETARG_TEXT_PP(0);++ if (PG_ARGISNULL(1))+ esc_text = NULL; /* use default escape character */+ else+ esc_text = PG_GETARG_TEXT_PP(1);++ result = similar_escape_internal(pat_text, esc_text);+
PG_RETURN_TEXT_P(result);
}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.datindex 8733524..15045d0 100644--- a/src/include/catalog/pg_proc.dat+++ b/src/include/catalog/pg_proc.dat@@ -3321,9 +3321,15 @@
proname => 'repeat', prorettype => 'text', proargtypes => 'text int4',
prosrc => 'repeat' },
-{ oid => '1623', descr => 'convert SQL99 regexp pattern to POSIX style',+{ oid => '1623', descr => 'convert SQL regexp pattern to POSIX style',
proname => 'similar_escape', proisstrict => 'f', prorettype => 'text',
proargtypes => 'text text', prosrc => 'similar_escape' },
+{ oid => '1986', descr => 'convert SQL regexp pattern to POSIX style',+ proname => 'similar_to_escape', prorettype => 'text',+ proargtypes => 'text text', prosrc => 'similar_to_escape_2' },+{ oid => '1987', descr => 'convert SQL regexp pattern to POSIX style',+ proname => 'similar_to_escape', prorettype => 'text', proargtypes => 'text',+ prosrc => 'similar_to_escape_1' },
{ oid => '1624',
proname => 'mul_d_interval', prorettype => 'interval',
@@ -5743,10 +5749,10 @@
{ oid => '2073', descr => 'extract text matching regular expression',
proname => 'substring', prorettype => 'text', proargtypes => 'text text',
prosrc => 'textregexsubstr' },
-{ oid => '2074', descr => 'extract text matching SQL99 regular expression',+{ oid => '2074', descr => 'extract text matching SQL regular expression',
proname => 'substring', prolang => 'sql', prorettype => 'text',
proargtypes => 'text text text',
- prosrc => 'select pg_catalog.substring($1, pg_catalog.similar_escape($2, $3))' },+ prosrc => 'select pg_catalog.substring($1, pg_catalog.similar_to_escape($2, $3))' },
{ oid => '2075', descr => 'convert int8 to bitstring',
proname => 'bit', prorettype => 'bit', proargtypes => 'int8 int4',
diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.outindex 486c00b..2483966 100644--- a/src/test/regress/expected/strings.out+++ b/src/test/regress/expected/strings.out@@ -410,7 +410,56 @@ SELECT SUBSTRING('abcdefg' FROM 'b(.*)f') AS "cde";
cde
(1 row)
--- PostgreSQL extension to allow using back reference in replace string;+-- Check behavior of SIMILAR TO, which uses largely the same regexp variant+SELECT 'abcdefg' SIMILAR TO '_bcd%' AS true;+ true +------+ t+(1 row)++SELECT 'abcdefg' SIMILAR TO 'bcd%' AS false;+ false +-------+ f+(1 row)++SELECT 'abcdefg' SIMILAR TO '_bcd#%' ESCAPE '#' AS false;+ false +-------+ f+(1 row)++SELECT 'abcd%' SIMILAR TO '_bcd#%' ESCAPE '#' AS true;+ true +------+ t+(1 row)++-- Postgres uses '\' as the default escape character, which is not per spec+SELECT 'abcdefg' SIMILAR TO '_bcd\%' AS false;+ false +-------+ f+(1 row)++-- and an empty string to mean "no escape", which is also not per spec+SELECT 'abcd\efg' SIMILAR TO '_bcd\%' ESCAPE '' AS true;+ true +------+ t+(1 row)++-- these behaviors are per spec, though:+SELECT 'abcdefg' SIMILAR TO '_bcd%' ESCAPE NULL AS null;+ null +------+ +(1 row)++SELECT 'abcdefg' SIMILAR TO '_bcd#%' ESCAPE '##' AS error;+ERROR: invalid escape string+HINT: Escape string must be empty or one character.+-- Test back reference in regexp_replace
SELECT regexp_replace('1112223333', E'(\\d{3})(\\d{3})(\\d{4})', E'(\\1) \\2-\\3');
regexp_replace
----------------
diff --git a/src/test/regress/sql/strings.sql b/src/test/regress/sql/strings.sqlindex 5744c9f..b5e75c3 100644--- a/src/test/regress/sql/strings.sql+++ b/src/test/regress/sql/strings.sql@@ -144,7 +144,20 @@ SELECT SUBSTRING('abcdefg' FROM 'c.e') AS "cde";
-- With a parenthesized subexpression, return only what matches the subexpr
SELECT SUBSTRING('abcdefg' FROM 'b(.*)f') AS "cde";
--- PostgreSQL extension to allow using back reference in replace string;+-- Check behavior of SIMILAR TO, which uses largely the same regexp variant+SELECT 'abcdefg' SIMILAR TO '_bcd%' AS true;+SELECT 'abcdefg' SIMILAR TO 'bcd%' AS false;+SELECT 'abcdefg' SIMILAR TO '_bcd#%' ESCAPE '#' AS false;+SELECT 'abcd%' SIMILAR TO '_bcd#%' ESCAPE '#' AS true;+-- Postgres uses '\' as the default escape character, which is not per spec+SELECT 'abcdefg' SIMILAR TO '_bcd\%' AS false;+-- and an empty string to mean "no escape", which is also not per spec+SELECT 'abcd\efg' SIMILAR TO '_bcd\%' ESCAPE '' AS true;+-- these behaviors are per spec, though:+SELECT 'abcdefg' SIMILAR TO '_bcd%' ESCAPE NULL AS null;+SELECT 'abcdefg' SIMILAR TO '_bcd#%' ESCAPE '##' AS error;++-- Test back reference in regexp_replace
SELECT regexp_replace('1112223333', E'(\\d{3})(\\d{3})(\\d{4})', E'(\\1) \\2-\\3');
SELECT regexp_replace('AAA BBB CCC ', E'\\s+', ' ', 'g');
SELECT regexp_replace('AAA', '^|$', 'Z', 'g');
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, andrew@tao11.riddles.org.uk, pgsql-hackers@lists.postgresql.org
Subject: Re: SQL-spec incompatibilities in similar_escape() and related stuff
In-Reply-To: <32617.1558649424@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