From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: SIMILAR TO expressions translate wildcards where they shouldn't
Date: Fri, 23 May 2025 11:42:10 +0200
Message-ID: <5498224d1d5a6a3be7f29ae7725f76162d48372f.camel@cybertec.at> (raw)
In-Reply-To: <aC_p7b5H6pLICU_e@paquier.xyz>
References: <16ab039d1af455652bdf4173402ddda145f2c73b.camel@cybertec.at>
<aC_K7JPXF0AdPZqB@paquier.xyz>
<aC_p7b5H6pLICU_e@paquier.xyz>
On Fri, 2025-05-23 at 12:22 +0900, Michael Paquier wrote:
> Anyway, I don't think that the tests in the patch are complete. For
> example, '[[^](]' is transformed into '^(?:[[^](])$' in the SIMILAR TO
> conversion with the patch, and before the patch we get
> '^(?:[[^](?:])$'. Note the translation of the last parenthesis '(' to
> "(?:" when inside the character class, but your patch does not
> document that. AFAIU, we should not convert the parenthesis '(' while
> in a multi-level character class as the patch does, but we have no
> tests for it and the patch does not document this part, either.
>
> Could it be possible to split the single SIMILAR TO expression into
> multiple smaller pieces for each character that should not be
> converted while inside a character class? This is hard to parse as
> written in your proposal of patch.
I have changed the regression test like you suggest.
I also improved the code by adding more comments.
I renamed "incharclass" to "charclass_depth", which is more descriptive.
Also, I had to work some more on handling carets:
While the closing bracket is a regular character in []] and [^]], it
is not in expressions like [^^].
Yours,
Laurenz Albe
Attachments:
[text/x-patch] v2-0001-Fix-SIMILAR-TO-regex-translation-for-character-cl.patch (5.0K, ../5498224d1d5a6a3be7f29ae7725f76162d48372f.camel@cybertec.at/2-v2-0001-Fix-SIMILAR-TO-regex-translation-for-character-cl.patch)
download | inline diff:
From f0c56f1f95ab35dca3d7ff65f783289a6b225146 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Fri, 23 May 2025 11:35:32 +0200
Subject: [PATCH v2] Fix SIMILAR TO regex translation for character classes
The code that translates SIMILAR TO pattern matching expressions
to regular expressions didn't consider that brackets can be nested,
as in [[:alpha:]%], and replaced placeholders like _ and % where
it shouldn't.
Fix by tracking the nesting level of brackets while considering that
in expressions like []] and [^]] the first closing bracket is a
regular character.
Author: Laurenz Albe <laurenz.albe@cybertec.at>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/16ab039d1af455652bdf4173402ddda145f2c73b.camel@cybertec.at
Backpatch-through: 13
---
src/backend/utils/adt/regexp.c | 36 ++++++++++++++++++++++-----
src/test/regress/expected/strings.out | 12 +++++++++
src/test/regress/sql/strings.sql | 3 +++
3 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/adt/regexp.c b/src/backend/utils/adt/regexp.cindex edee1f7880b..9d6f7675c64 100644--- a/src/backend/utils/adt/regexp.c+++ b/src/backend/utils/adt/regexp.c@@ -773,7 +773,8 @@ similar_escape_internal(text *pat_text, text *esc_text)
int plen,
elen;
bool afterescape = false;
- bool incharclass = false;+ int charclass_depth = 0; /* sqare bracket nesting level */+ int charclass_start = 0;
int nquotes = 0;
p = VARDATA_ANY(pat_text);
@@ -904,7 +905,7 @@ similar_escape_internal(text *pat_text, text *esc_text)
/* fast path */
if (afterescape)
{
- if (pchar == '"' && !incharclass) /* escape-double-quote? */+ if (pchar == '"' && charclass_depth < 1) /* escape-double-quote? */
{
/* emit appropriate part separator, per notes above */
if (nquotes == 0)
@@ -953,18 +954,41 @@ similar_escape_internal(text *pat_text, text *esc_text)
/* SQL escape character; do not send to output */
afterescape = true;
}
- else if (incharclass)+ else if (charclass_depth > 0)
{
if (pchar == '\\')
*r++ = '\\';
*r++ = pchar;
- if (pchar == ']')- incharclass = false;++ /*+ * Ignore a closing bracket at the start of a character class.+ * Such a bracket is taken literally rather than closing the+ * class. "charclass_start" is 1 right at the beginning of a class+ * and 2 after an initial caret.+ */+ if (pchar == '[')+ charclass_depth++;+ else if (pchar == ']' && charclass_start > 2)+ charclass_depth--;++ /*+ * If there is a caret right after the opening bracket, it negates+ * the character class, but a following closing bracket should+ * still be treated as a normal character. However, that holds+ * only for the first caret, so only the values 1 and 2 mean that+ * closing brackets should be taken literally.+ */+ if (pchar == '^')+ charclass_start++;+ else+ charclass_start = 3; /* definitely past the start */
}
else if (pchar == '[')
{
+ /* start of a character class */
*r++ = pchar;
- incharclass = true;+ charclass_depth++;+ charclass_start = 1;
}
else if (pchar == '%')
{
diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.outindex 174f0a68331..ce230961b5c 100644--- a/src/test/regress/expected/strings.out+++ b/src/test/regress/expected/strings.out@@ -614,6 +614,18 @@ SELECT 'abcdefg' SIMILAR TO '_bcd%' ESCAPE NULL AS null;
SELECT 'abcdefg' SIMILAR TO '_bcd#%' ESCAPE '##' AS error;
ERROR: invalid escape string
HINT: Escape string must be empty or one character.
+-- ".", "_", "%", "$", "(" and "^" should be left alone in character classes+EXPLAIN (VERBOSE, COSTS OFF)+SELECT (SELECT '') SIMILAR TO '.[.[:alnum:]_]_[]%]%[^]$]$[^^]^[(](p)';+ QUERY PLAN +-------------------------------------------------------------------------------------------+ Result+ Output: ((InitPlan 1).col1 ~ '^(?:\.[.[:alnum:]_].[]%].*[^]$]\$[^^]\^[(](?:p))$'::text)+ InitPlan 1+ -> Result+ Output: ''::text+(5 rows)+
-- Test backslash escapes in regexp_replace's replacement string
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 f7b325baadf..94b9aa48e90 100644--- a/src/test/regress/sql/strings.sql+++ b/src/test/regress/sql/strings.sql@@ -196,6 +196,9 @@ 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;
+-- ".", "_", "%", "$", "(" and "^" should be left alone in character classes+EXPLAIN (VERBOSE, COSTS OFF)+SELECT (SELECT '') SIMILAR TO '.[.[:alnum:]_]_[]%]%[^]$]$[^^]^[(](p)';
-- Test backslash escapes in regexp_replace's replacement string
SELECT regexp_replace('1112223333', E'(\\d{3})(\\d{3})(\\d{4})', E'(\\1) \\2-\\3');
--
2.49.0
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-bugs@postgresql.org
Cc: laurenz.albe@cybertec.at, michael@paquier.xyz, pgsql-bugs@lists.postgresql.org
Subject: Re: SIMILAR TO expressions translate wildcards where they shouldn't
In-Reply-To: <5498224d1d5a6a3be7f29ae7725f76162d48372f.camel@cybertec.at>
* 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