pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedSIMILAR TO expressions translate wildcards where they shouldn't
11+ messages / 3 participants
[nested] [flat]
* SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-22 21:18 Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 11+ messages in thread
From: Laurenz Albe @ 2025-05-22 21:18 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org
The following surprising result
SELECT 'a_b' SIMILAR TO '[_[:alpha:]]*',
'a_b' SIMILAR TO '[[:alpha:]_]*';
?column? │ ?column?
══════════╪══════════
t │ f
(1 row)
becomes clear when we look how the expressions are translated to
regular expressions:
EXPLAIN (VERBOSE, GENERIC_PLAN, COSTS OFF)
SELECT $1 SIMILAR TO '[_[:alpha:]]*',
$1 SIMILAR TO '[[:alpha:]_]*';
QUERY PLAN
══════════════════════════════════════════════════════════════════════════════════
Result
Output: ($1 ~ '^(?:[_[:alpha:]]*)$'::text), ($1 ~ '^(?:[[:alpha:].]*)$'::text)
(2 rows)
The underscore before the [:alpha:] is left alone, but the one after
it gets translated to a period. Now the underscore is a wildcard
that corresponds to the period in regular expressions, but characters
in square brackets should lose their special meaning. The code in
utils/adt/regexp.c doesn't expect that square brackets can be nested.
The attached patch fixes the bug.
Yours,
Laurenz Albe
Attachments:
[text/x-patch] v1-0001-Fix-SIMILAR-TO-regex-translation.patch (3.9K, ../../16ab039d1af455652bdf4173402ddda145f2c73b.camel@cybertec.at/2-v1-0001-Fix-SIMILAR-TO-regex-translation.patch)
download | inline diff:
From df3f0c76eec2314ab7a23ef99d4e390d2456a1ce Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Thu, 22 May 2025 23:05:30 +0200
Subject: [PATCH v1] Fix SIMILAR TO regex translation
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.
---
src/backend/utils/adt/regexp.c | 19 +++++++++++++------
src/test/regress/expected/strings.out | 12 ++++++++++++
src/test/regress/sql/strings.sql | 3 +++
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/adt/regexp.c b/src/backend/utils/adt/regexp.c
index edee1f7880b..a23c4c959e4 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 incharclass = 0;
+ bool charclass_start = false;
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 == '"' && incharclass < 1) /* escape-double-quote? */
{
/* emit appropriate part separator, per notes above */
if (nquotes == 0)
@@ -953,18 +954,24 @@ similar_escape_internal(text *pat_text, text *esc_text)
/* SQL escape character; do not send to output */
afterescape = true;
}
- else if (incharclass)
+ else if (incharclass > 0)
{
if (pchar == '\\')
*r++ = '\\';
*r++ = pchar;
- if (pchar == ']')
- incharclass = false;
+ if (pchar == '[')
+ incharclass++;
+ else if (pchar == ']' && !charclass_start)
+ incharclass--;
+
+ if (pchar != '^' && charclass_start)
+ charclass_start = false;
}
else if (pchar == '[')
{
*r++ = pchar;
- incharclass = true;
+ incharclass++;
+ charclass_start = true;
}
else if (pchar == '%')
{
diff --git a/src/test/regress/expected/strings.out b/src/test/regress/expected/strings.out
index 174f0a68331..aa0bddd682c 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:]._]*._[]._]._[^].%].%';
+ QUERY PLAN
+----------------------------------------------------------------------------------------
+ Result
+ Output: ((InitPlan 1).col1 ~ '^(?:\..[._[:alnum:]._]*\..[]._]\..[^].%]\..*)$'::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.sql
index f7b325baadf..eb743d9e3af 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:]._]*._[]._]._[^].%].%';
-- 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
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-23 01:10 Michael Paquier <michael@paquier.xyz>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 11+ messages in thread
From: Michael Paquier @ 2025-05-23 01:10 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: pgsql-bugs@lists.postgresql.org
On Thu, May 22, 2025 at 11:18:44PM +0200, Laurenz Albe wrote:
> The underscore before the [:alpha:] is left alone, but the one after
> it gets translated to a period. Now the underscore is a wildcard
> that corresponds to the period in regular expressions, but characters
> in square brackets should lose their special meaning. The code in
> utils/adt/regexp.c doesn't expect that square brackets can be nested.
>
> The attached patch fixes the bug.
Oh, good catch. [_[:alpha:]] and [[:alpha:]_] both that this should
match every string made of a-zA-Z and underscores, but this is failing
to do the job for the latter.
+ if (pchar != '^' && charclass_start)
+ charclass_start = false;
I'm a bit puzzled by this part about '^', though, resetting the fact
that we are in a squared bracket section with '^' treated as an
exception. Perhaps this deserves a comment?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aC_K7JPXF0AdPZqB@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-23 03:22 Michael Paquier <michael@paquier.xyz>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 11+ messages in thread
From: Michael Paquier @ 2025-05-23 03:22 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: pgsql-bugs@lists.postgresql.org
On Fri, May 23, 2025 at 10:10:04AM +0900, Michael Paquier wrote:
> Oh, good catch. [_[:alpha:]] and [[:alpha:]_] both that this should
> match every string made of a-zA-Z and underscores, but this is failing
> to do the job for the latter.
>
> + if (pchar != '^' && charclass_start)
> + charclass_start = false;
>
> I'm a bit puzzled by this part about '^', though, resetting the fact
> that we are in a squared bracket section with '^' treated as an
> exception. Perhaps this deserves a comment?
Ah, I see. This is just a way of saying that the caret ^ should still
be able to track the first character in the character class.
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.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aC_p7b5H6pLICU_e@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-23 09:42 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 11+ messages in thread
From: Laurenz Albe @ 2025-05-23 09:42 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-bugs@lists.postgresql.org
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.c
index 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.out
index 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.sql
index 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
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-27 05:57 Michael Paquier <michael@paquier.xyz>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 11+ messages in thread
From: Michael Paquier @ 2025-05-27 05:57 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: pgsql-bugs@lists.postgresql.org
On Fri, May 23, 2025 at 11:42:10AM +0200, Laurenz Albe wrote:
> 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 [^^].
The transformation with the patch:
'^(?:\.[.[:alnum:]_].[]%].*[^]$]\$[^^]\^[(](?:p))$'
And on HEAD:
'^(?:\.[.[:alnum:].].[].*].*[^]\$]\$[^^]\^[(](?:p))$'
If I am not missing something, '%', '_', '$', single and double carets
at the beginning are covered. '.' and '(' are not.
Anyway, that's really hard to parse so I would suggest to split each
check into queries of their own to show individual conversions in
these EXPLAIN outputs (we don't care if tese regexps are correct, just
want to check the output to the POSIX style). I am OK with the point
based on charclass_start to count the number of carets at the
beginning of a character class.
With some tweaks and the tests reworked, I am finishing with the
reviewed version attached. What do you think?
--
Michael
Attachments:
[text/x-diff] v3-0001-Fix-SIMILAR-TO-regex-translation-for-character-cl.patch (8.8K, ../../aDVUWaoZg8cW3B9x@paquier.xyz/2-v3-0001-Fix-SIMILAR-TO-regex-translation-for-character-cl.patch)
download | inline diff:
From a8cf529a620201b6cb0bff179f10a008ace8a904 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 27 May 2025 14:41:21 +0900
Subject: [PATCH v3] 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 | 38 +++++++++--
src/test/regress/expected/strings.out | 91 +++++++++++++++++++++++++++
src/test/regress/sql/strings.sql | 20 ++++++
3 files changed, 143 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/adt/regexp.c b/src/backend/utils/adt/regexp.c
index edee1f7880bd..6e2864cbbda8 100644
--- a/src/backend/utils/adt/regexp.c
+++ b/src/backend/utils/adt/regexp.c
@@ -773,8 +773,11 @@ similar_escape_internal(text *pat_text, text *esc_text)
int plen,
elen;
bool afterescape = false;
- bool incharclass = false;
int nquotes = 0;
+ int charclass_depth = 0; /* Nesting level of character classes,
+ * encompassed by square brackets */
+ int charclass_start = 0; /* State of the character class start,
+ * for carets */
p = VARDATA_ANY(pat_text);
plen = VARSIZE_ANY_EXHDR(pat_text);
@@ -904,7 +907,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 +956,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_start > 2)
+ charclass_depth--;
+ else if (pchar == '[')
+ 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. 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.out
index 174f0a68331b..601bc7d2ec25 100644
--- a/src/test/regress/expected/strings.out
+++ b/src/test/regress/expected/strings.out
@@ -614,6 +614,97 @@ 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.
+-- Characters that should be left alone in character classes when a
+-- SIMILAR TO regexp pattern is converted to POSIX style.
+-- Underscore "_"
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '_[_[:alpha:]_]_';
+ QUERY PLAN
+---------------------------------------------------------------
+ Result
+ Output: ((InitPlan 1).col1 ~ '^(?:.[_[:alpha:]_].)$'::text)
+ InitPlan 1
+ -> Result
+ Output: ''::text
+(5 rows)
+
+-- Percentage "%"
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '%[%[:alnum:]%]%';
+ QUERY PLAN
+-----------------------------------------------------------------
+ Result
+ Output: ((InitPlan 1).col1 ~ '^(?:.*[%[:alnum:]%].*)$'::text)
+ InitPlan 1
+ -> Result
+ Output: ''::text
+(5 rows)
+
+-- Dot "."
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '.[.[:alnum:].].';
+ QUERY PLAN
+-----------------------------------------------------------------
+ Result
+ Output: ((InitPlan 1).col1 ~ '^(?:\.[.[:alnum:].]\.)$'::text)
+ InitPlan 1
+ -> Result
+ Output: ''::text
+(5 rows)
+
+-- Dollar "$"
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '$[$[:alnum:]$]$';
+ QUERY PLAN
+-----------------------------------------------------------------
+ Result
+ Output: ((InitPlan 1).col1 ~ '^(?:\$[$[:alnum:]$]\$)$'::text)
+ InitPlan 1
+ -> Result
+ Output: ''::text
+(5 rows)
+
+-- Opening parenthesis "("
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '([([:alnum:](](';
+ QUERY PLAN
+-------------------------------------------------------------------
+ Result
+ Output: ((InitPlan 1).col1 ~ '^(?:(?:[([:alnum:](](?:)$'::text)
+ InitPlan 1
+ -> Result
+ Output: ''::text
+(5 rows)
+
+-- Caret "^"
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '^[^[:alnum:]^[^^][[^^]][\^][[\^]]\^]^';
+ QUERY PLAN
+---------------------------------------------------------------------------------------
+ Result
+ Output: ((InitPlan 1).col1 ~ '^(?:\^[^[:alnum:]^[^^][[^^]][\^][[\^]]\^]\^)$'::text)
+ InitPlan 1
+ -> Result
+ Output: ''::text
+(5 rows)
+
+-- Closing square bracket "]" at the beginning of character class
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '[]%][^]%][^%]%';
+ QUERY PLAN
+---------------------------------------------------------------
+ Result
+ Output: ((InitPlan 1).col1 ~ '^(?:[]%][^]%][^%].*)$'::text)
+ InitPlan 1
+ -> Result
+ Output: ''::text
+(5 rows)
+
+-- Closing square bracket effective after two carets at the beginning
+-- of character class.
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '[^^]^';
+ QUERY PLAN
+------------------------------------------------------
+ Result
+ Output: ((InitPlan 1).col1 ~ '^(?:[^^]\^)$'::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.sql
index f7b325baadf4..34c52b23604b 100644
--- a/src/test/regress/sql/strings.sql
+++ b/src/test/regress/sql/strings.sql
@@ -197,6 +197,26 @@ SELECT 'abcd\efg' SIMILAR TO '_bcd\%' ESCAPE '' AS true;
SELECT 'abcdefg' SIMILAR TO '_bcd%' ESCAPE NULL AS null;
SELECT 'abcdefg' SIMILAR TO '_bcd#%' ESCAPE '##' AS error;
+-- Characters that should be left alone in character classes when a
+-- SIMILAR TO regexp pattern is converted to POSIX style.
+-- Underscore "_"
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '_[_[:alpha:]_]_';
+-- Percentage "%"
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '%[%[:alnum:]%]%';
+-- Dot "."
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '.[.[:alnum:].].';
+-- Dollar "$"
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '$[$[:alnum:]$]$';
+-- Opening parenthesis "("
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '([([:alnum:](](';
+-- Caret "^"
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '^[^[:alnum:]^[^^][[^^]][\^][[\^]]\^]^';
+-- Closing square bracket "]" at the beginning of character class
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '[]%][^]%][^%]%';
+-- Closing square bracket effective after two carets at the beginning
+-- of character class.
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '[^^]^';
+
-- Test backslash escapes in regexp_replace's replacement string
SELECT regexp_replace('1112223333', E'(\\d{3})(\\d{3})(\\d{4})', E'(\\1) \\2-\\3');
SELECT regexp_replace('foobarrbazz', E'(.)\\1', E'X\\&Y', 'g');
--
2.49.0
[application/pgp-signature] signature.asc (832B, ../../aDVUWaoZg8cW3B9x@paquier.xyz/3-signature.asc)
download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-27 10:24 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 11+ messages in thread
From: Laurenz Albe @ 2025-05-27 10:24 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: pgsql-bugs@lists.postgresql.org
On Tue, 2025-05-27 at 14:57 +0900, Michael Paquier wrote:
> Anyway, that's really hard to parse so I would suggest to split each
> check into queries of their own to show individual conversions in
> these EXPLAIN outputs (we don't care if tese regexps are correct, just
> want to check the output to the POSIX style). I am OK with the point
> based on charclass_start to count the number of carets at the
> beginning of a character class.
>
> With some tweaks and the tests reworked, I am finishing with the
> reviewed version attached. What do you think?
Thank you; I think that is good to go.
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-27 14:54 Tom Lane <tgl@sss.pgh.pa.us>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2025-05-27 14:54 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: Michael Paquier <michael@paquier.xyz>; pgsql-bugs@lists.postgresql.org
Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Tue, 2025-05-27 at 14:57 +0900, Michael Paquier wrote:
>> With some tweaks and the tests reworked, I am finishing with the
>> reviewed version attached. What do you think?
> Thank you; I think that is good to go.
Code changes look good, but I think the test cases are too cute:
+EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '_[_[:alpha:]_]_';
+ QUERY PLAN
+---------------------------------------------------------------
+ Result
+ Output: ((InitPlan 1).col1 ~ '^(?:.[_[:alpha:]_].)$'::text)
+ InitPlan 1
+ -> Result
+ Output: ''::text
+(5 rows)
This will break whenever somebody decides it's worth optimizing
a sub-select that looks like that. I'd suggest following the
pattern
explain (costs off) select * from text_tbl where f1 similar to 'z';
QUERY PLAN
----------------------------------
Seq Scan on text_tbl
Filter: (f1 ~ '^(?:z)$'::text)
(2 rows)
which is both less noisy and less likely to change in future.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-27 21:39 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 11+ messages in thread
From: Laurenz Albe @ 2025-05-27 21:39 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Michael Paquier <michael@paquier.xyz>; pgsql-bugs@lists.postgresql.org
On Tue, 2025-05-27 at 10:54 -0400, Tom Lane wrote:
> Laurenz Albe <laurenz.albe@cybertec.at> writes:
> > On Tue, 2025-05-27 at 14:57 +0900, Michael Paquier wrote:
> > > With some tweaks and the tests reworked, I am finishing with the
> > > reviewed version attached. What do you think?
>
> > Thank you; I think that is good to go.
>
> Code changes look good, but I think the test cases are too cute:
>
> +EXPLAIN (VERBOSE, COSTS OFF) SELECT (SELECT '') SIMILAR TO '_[_[:alpha:]_]_';
> + QUERY PLAN
> +---------------------------------------------------------------
> + Result
> + Output: ((InitPlan 1).col1 ~ '^(?:.[_[:alpha:]_].)$'::text)
> + InitPlan 1
> + -> Result
> + Output: ''::text
> +(5 rows)
>
> This will break whenever somebody decides it's worth optimizing
> a sub-select that looks like that. I'd suggest following the
> pattern
>
> explain (costs off) select * from text_tbl where f1 similar to 'z';
> QUERY PLAN
> ----------------------------------
> Seq Scan on text_tbl
> Filter: (f1 ~ '^(?:z)$'::text)
> (2 rows)
>
> which is both less noisy and less likely to change in future.
That's a good point.
I originally considered EXPLAIN (GENERIC_PLAN), but that would only
backpatch so far.
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-28 00:00 Michael Paquier <michael@paquier.xyz>
parent: Laurenz Albe <laurenz.albe@cybertec.at>
0 siblings, 2 replies; 11+ messages in thread
From: Michael Paquier @ 2025-05-28 00:00 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs@lists.postgresql.org
On Tue, May 27, 2025 at 11:39:02PM +0200, Laurenz Albe wrote:
> I originally considered EXPLAIN (GENERIC_PLAN), but that would only
> backpatch so far.
Nice trick that makes the test output much smaller. I've used this
idea and applied the fix down to v13 after a second lookup this
morning.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aDZSFLHbohoLJQLd@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-28 00:15 Michael Paquier <michael@paquier.xyz>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 0 replies; 11+ messages in thread
From: Michael Paquier @ 2025-05-28 00:15 UTC (permalink / raw)
To: Laurenz Albe <laurenz.albe@cybertec.at>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs@lists.postgresql.org
On Wed, May 28, 2025 at 09:00:20AM +0900, Michael Paquier wrote:
> Nice trick that makes the test output much smaller. I've used this
> idea and applied the fix down to v13 after a second lookup this
> morning.
And I've managed to miss that the case with an opening parenthesis
throws an error due to an unbalanced set of parentheses because the
regexp is processed when the query is written the way Tom has
mentioned.
Will fix in a bit after an extra round of check-world, sorry about
that..
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../aDZVpfnWONm10F35@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: SIMILAR TO expressions translate wildcards where they shouldn't
@ 2025-05-28 08:33 Laurenz Albe <laurenz.albe@cybertec.at>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 0 replies; 11+ messages in thread
From: Laurenz Albe @ 2025-05-28 08:33 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-bugs@lists.postgresql.org
On Wed, 2025-05-28 at 09:00 +0900, Michael Paquier wrote:
> On Tue, May 27, 2025 at 11:39:02PM +0200, Laurenz Albe wrote:
> > I originally considered EXPLAIN (GENERIC_PLAN), but that would only
> > backpatch so far.
>
> Nice trick that makes the test output much smaller. I've used this
> idea and applied the fix down to v13 after a second lookup this
> morning.
Thanks for taking care of this!
Yours,
Laurenz Albe
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2025-05-28 08:33 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-05-22 21:18 SIMILAR TO expressions translate wildcards where they shouldn't Laurenz Albe <laurenz.albe@cybertec.at>
2025-05-23 01:10 ` Michael Paquier <michael@paquier.xyz>
2025-05-23 03:22 ` Michael Paquier <michael@paquier.xyz>
2025-05-23 09:42 ` Laurenz Albe <laurenz.albe@cybertec.at>
2025-05-27 05:57 ` Michael Paquier <michael@paquier.xyz>
2025-05-27 10:24 ` Laurenz Albe <laurenz.albe@cybertec.at>
2025-05-27 14:54 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-05-27 21:39 ` Laurenz Albe <laurenz.albe@cybertec.at>
2025-05-28 00:00 ` Michael Paquier <michael@paquier.xyz>
2025-05-28 00:15 ` Michael Paquier <michael@paquier.xyz>
2025-05-28 08:33 ` Laurenz Albe <laurenz.albe@cybertec.at>
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