Enable regular expressions containing \s and \S

[sperlingxx]

Closes #4528

Signed-off-by: sperlingxx lovedreamf@gmail.com

Enable regular expressions which contain \s and \S and provide corresponding tests.

[sperlingxx]

build

[revans2]

It looks like there may be a version mismatch or something.

[2022-03-30T12:27:10.759Z]     def deco(*a, **kw):
[2022-03-30T12:27:10.759Z]         try:
[2022-03-30T12:27:10.759Z]             return f(*a, **kw)
[2022-03-30T12:27:10.759Z]         except py4j.protocol.Py4JJavaError as e:
[2022-03-30T12:27:10.759Z]             converted = convert_exception(e.java_exception)
[2022-03-30T12:27:10.760Z]             if not isinstance(converted, UnknownException):
[2022-03-30T12:27:10.760Z]                 # Hide where the exception came from that shows a non-Pythonic
[2022-03-30T12:27:10.760Z]                 # JVM exception message.
[2022-03-30T12:27:10.760Z] >               raise converted from None
[2022-03-30T12:27:10.760Z] E               pyspark.sql.utils.AnalysisException: Undefined function: 'regexp'. This function is neither a registered temporary function nor a permanent function registered in the database 'default'.; line 1 pos 0
[2022-03-30T12:27:10.760Z] 
[2022-03-30T12:27:10.760Z] ../../../.download/spark-3.1.1-bin-hadoop3.2/python/pyspark/sql/utils.py:117: AnalysisException
...
FAILED ../../src/main/python/string_test.py::test_regexp_whitespace - pyspark...

[revans2]

The change looks fine to me, but I want to be sure that the test is good enough to cover the corner cases we expect. @sperlingxx did you check the CUDF implementation to be sure that they match? @andygrove Are there other characters that you would like to see in there? Especially in we want to catch possible cudf regressions.

[andygrove]

Could you also update RegularExpressionTranspilerSuite.REGEXP_LIMITED_CHARS_COMMON to add \f and \u000b to the list of characters used by the fuzzer tests. That should be enough to see if there are any edge cases not covered here.

[andygrove]

I couldn't find documentation for what cuDF considers whitespace but through experimentation in the fuzz tests, I found that cuDF appears to consider\u001e (record separator) to be a whitespace but Java does not. Here is a test that can be added to RegularExpressionTranspilerSuite to demonstrate this.

  test("whitespace boundaries - replace") {
    val patterns = Seq("\\s", "\\S")
    val inputs = Seq("\u001eTEST")
    assertCpuGpuMatchesRegexpReplace(patterns, inputs)
  }

This fails with javaPattern=\s, cudfPattern=\s, input='�TEST', cpu=�TEST, gpu=_RE\PLACE_TEST.

[andygrove]

Rather than pass \s and \S to cuDF, we should transpile to a pattern that would guarantee compatibility with Java, so something like [ \f\t\r\n\\x0b].

[sperlingxx]

Rather than pass \s and \S to cuDF, we should transpile to a pattern that would guarantee compatibility with Java, so something like [ \f\t\r\n\\x0b].

Yes, I changed the parser to transpile \s as [ \f\t\r\n\\x0b].

[sperlingxx]

build

[andygrove]

LGTM. I don't think we need to change the Spark2 code but I don't think it hurts either.