Support some escape chars when rewriting regexp_replace to stringReplace #11813
Conversation
…eplace to string replace Signed-off-by: Haoyang Li <[email protected]>
| @@ -1032,7 +1034,8 @@ def test_regexp_replace_multi_optimization(regexp_enabled): | |||
| 'REGEXP_REPLACE(a, "aa|bb|cc|dd", "PROD")', | |||
| 'REGEXP_REPLACE(a, "(aa|bb)|(cc|dd)", "PROD")', | |||
| 'REGEXP_REPLACE(a, "aa|bb|cc|dd|ee", "PROD")', | |||
| 'REGEXP_REPLACE(a, "aa|bb|cc|dd|ee|ff", "PROD")' | |||
| 'REGEXP_REPLACE(a, "aa|bb|cc|dd|ee|ff", "PROD")', | |||
| 'REGEXP_REPLACE(a, "a\n|b\a|c\t", "PROD")' | |||
There was a problem hiding this comment.
gen = mk_str_gen('[abcdef\t]{0,3}')
We can include some of the supported characters in the input sample and also consider regex patterns like a\ta|b\nb, where the supported characters aren't exclusively at the end.
There was a problem hiding this comment.
Thanks! I think we may also need to change gen to something like gen = mk_str_gen('[abcdef\t]{0,3}') to include supported characters in the input.
There was a problem hiding this comment.
oh I missed that, thanks, updated.
| @@ -593,8 +593,9 @@ object GpuOverrides extends Logging { | |||
| } | |||
|
|
|||
| def isSupportedStringReplacePattern(strLit: String): Boolean = { | |||
| // check for regex special characters, except for \u0000 which we can support | |||
| !regexList.filterNot(_ == "\u0000").exists(pattern => strLit.contains(pattern)) | |||
| // check for regex special characters, except for \u0000, \n, \r, and \t which we can support | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Deleted this comment as it is duplicated and intended to describe the internal logic of this function.
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
Signed-off-by: Haoyang Li <[email protected]>
|
build |
| @@ -593,8 +593,9 @@ object GpuOverrides extends Logging { | |||
| } | |||
|
|
|||
| def isSupportedStringReplacePattern(strLit: String): Boolean = { | |||
| // check for regex special characters, except for \u0000 which we can support | |||
| !regexList.filterNot(_ == "\u0000").exists(pattern => strLit.contains(pattern)) | |||
| // check for regex special characters, except for \u0000, \n, \r, and \t which we can support | |||
There was a problem hiding this comment.
Just out of curiosity is there a reason we cannot support all \uXXXX escaped characters? It is just replaced with a unicode character and we should be able to do that directly.
There was a problem hiding this comment.
This line is to filter out regex special characters that can be replaced, normal utf8 characters are not in this list and are already supported.
Closes #11812
We have a performance improvement of regexp_replace to transpile simple choice-type regular expressions into lists of choices to use with string replace multi. But when the search list contains some escape characters like \n, \r and \t, we won't go that path.
This PR adds support for
\n,\rand\twhen rewriting regexp_replace to string replace and string replace multi.