[CALCITE-6278] Add REGEXP, REGEXP_LIKE function (enabled in Spark library) by YiwenWu · Pull Request #3712 · apache/calcite

YiwenWu · 2024-03-03T02:37:45Z

https://issues.apache.org/jira/browse/CALCITE-6278

Add Function REGEXP、REGEXP_LIKE enabled in the Spark library.
Since this function has the same implementation as the Spark RLIKE function.
The implementation can be reused.

Source Code

[undo] Discuss results in Jira:

Since Spark 2.0, string literals (including regex patterns) are unescaped in SQL parser, also fix this bug in calcite.

Add spark query tests in BabelQuidemTest to ensure the execution correctness.

caicancai · 2024-03-03T02:48:53Z

testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java

  }

+  @Test void testRegexpFunc() {
+    final SqlOperatorFixture f = fixture().setFor(SqlLibraryOperators.REGEXP);


fixture().setFor(SqlLibraryOperators.REGEXP).withLibrary(SqlLibrary.Spark) ？

withLibrary is optional.
f0.forEachLibrary(list(functionAlias.libraries), consumer) will automatically load the corresponding Library type.

caicancai · 2024-03-03T02:50:05Z

babel/src/test/resources/sql/spark.iq

@@ -0,0 +1,243 @@
+# spark.iq - Babel test for Spark dialect of SQL


I have a question.Are you sure this test only takes effect for semantics that conform to spark sql?

This test will load Built-in functions and spark functions.

I am not against using this test.

But I want to know the necessity of using QuidemTest for Spark.
Because your code may become a future contribution specification.

Let's discuss two questions.

Why need spark.iq?

What features need to be tested and what features don't need to be tested?

+1 At present, calcite has adapted many spark functions. Do we need to add all spark functions? What are the benefits after adding them?

Compared to SqlOperatorTest, I think the function tests in Babel are more accurate and complete. Therefore,
referencesbig-query.iq and redshift.iq, spark.iq were added to verify the correctness of function execution.

I think that unit tests can be added to this file when adding spark functions, but it is unnecessary.

caicancai · 2024-03-03T02:51:47Z

core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java

    /** SQL {@code RLIKE} function. */
    public boolean rlike(String s, String pattern) {
-      return cache.getUnchecked(new Key(0, pattern)).matcher(s).find();
+      s = StringEscapeUtils.unescapeJava(s);


Do we need to add a little comment?

done, add comment

julianhyde · 2024-03-04T21:02:12Z

babel/src/test/resources/sql/spark.iq

+# REGEXP(str, regexp)
+# Returns true if str matches regexp, or false otherwise.
+#
+# Returns STRING


no it doesn't

done, fixed to return Boolean

julianhyde · 2024-03-04T21:02:54Z

site/_docs/reference.md

 | b | PARSE_TIMESTAMP(format, string[, timeZone])    | Uses format specified by *format* to convert *string* representation of timestamp to a TIMESTAMP WITH LOCAL TIME ZONE value in *timeZone*
 | h s | PARSE_URL(urlString, partToExtract [, keyToExtract] ) | Returns the specified *partToExtract* from the *urlString*. Valid values for *partToExtract* include HOST, PATH, QUERY, REF, PROTOCOL, AUTHORITY, FILE, and USERINFO. *keyToExtract* specifies which query to extract
 | b s | POW(numeric1, numeric2)                      | Returns *numeric1* raised to the power *numeric2*
+| s | REGEXP(string, regexp)                         | Returns true if *string* matches *regexp*, or false otherwise, string literals (including regex patterns) are unescaped


can't you just say 'alias for RLIKE'?

done, Equivalent to string1 RLIKE string2

julianhyde · 2024-03-04T21:04:01Z

testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java

+      f.checkNull(fn + "(cast(null as varchar), 'abc')");
+      f.checkNull(fn + "(cast(null as varchar), cast(null as varchar))");
+    };
+    f0.forEachLibrary(list(functionAlias.libraries), consumer);


if RLIKE, REGEXP_LIKE and REGEXP are identical, you should devise a way to test all three with a single test

Although RLIKE and REGEXP have the same implementation, their function calling methods differ, so they are divided into different tests.

RLIKE: 'str' RLIKE 'regex'

REGEXP|REGEXP_LIKE : fn('str', 'regex').

In addition, we can abstract a method to support different Function calling modes, but I may think it is not necessary at present.

If you think it's better to combine them into a single test, I'll modify the test case.

It is necessary. And it's easy to combine all three. BinaryOperator<String>. For RLIKE, pass (a, b) -> a + " RLIKE " b; for REGEXP, pass (a, b) -> "REGEXP(" + a + ", " + b + ")".

done, combined test case of RLIKE、REGEXP、REGEXP_LIKE

macroguo-ghy · 2024-03-07T07:00:23Z

core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java

    /** SQL {@code RLIKE} function. */
    public boolean rlike(String s, String pattern) {
-      return cache.getUnchecked(new Key(0, pattern)).matcher(s).find();
+      // Since Spark 2.0, string literals (including regex patterns) are unescaped in SQL parser


rlike also enabled in HIVE library, does the rlike function have the same semantics in hive as in spark sql?

Yes, judging by the engine execution behavior, Hive also performs the unescape, and the result is consistent with Spark.

macroguo-ghy

+1

jduo · 2024-03-09T23:19:56Z

Would it be possible to merge these changes? I'm looking to implement CALCITE-6309 on top of this.

JiajunBernoulli · 2024-03-10T02:30:29Z

site/_docs/reference.md

 | b m p s | RIGHT(string, length)                    | Returns the rightmost *length* characters from the *string*
-| h s | string1 RLIKE string2                        | Whether *string1* matches regex pattern *string2* (similar to `LIKE`, but uses Java regex)
-| h s | string1 NOT RLIKE string2                    | Whether *string1* does not match regex pattern *string2* (similar to `NOT LIKE`, but uses Java regex)
+| h s | string1 RLIKE string2                        | Whether *string1* matches regex pattern *string2* (similar to `LIKE`, but uses Java regex), string literals (including regex patterns) are unescaped


Should we maintain alphabetical order?

re before than ri

done, order adjusted.

YiwenWu · 2024-03-10T06:07:31Z

Would it be possible to merge these changes? I'm looking to implement CALCITE-6309 on top of this.

@jduo I think this pr will be able to be merged recently, the details of the relevant issues have been discussed clearly in Jira.

…rary)

sonarqubecloud · 2024-03-10T06:30:22Z

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

macroguo-ghy · 2024-03-11T03:51:02Z

If no other comment, I will merge it later.

jduo · 2024-03-11T23:11:48Z

If no other comment, I will merge it later.

Thanks, that'd be great. Once that's in I'll follow-up with the CALCITE-6309 work.

YiwenWu force-pushed the spark_func_regexp branch from 20f4d1e to b4fa622 Compare March 3, 2024 02:48

YiwenWu changed the title ~~[CALCITE-6278] Add REGEXP function (enabled in Spark library)~~ [CALCITE-6278] Add REGEXP、REGEXP_LIKE function (enabled in Spark library) Mar 3, 2024

caicancai reviewed Mar 3, 2024

View reviewed changes

YiwenWu force-pushed the spark_func_regexp branch 2 times, most recently from d885c38 to d4bce2a Compare March 3, 2024 03:18

julianhyde reviewed Mar 4, 2024

View reviewed changes

YiwenWu force-pushed the spark_func_regexp branch 2 times, most recently from 1545b81 to 6161df5 Compare March 6, 2024 04:12

YiwenWu changed the title ~~[CALCITE-6278] Add REGEXP、REGEXP_LIKE function (enabled in Spark library)~~ [CALCITE-6278] Add REGEXP, REGEXP_LIKE function (enabled in Spark library) Mar 7, 2024

macroguo-ghy reviewed Mar 7, 2024

View reviewed changes

macroguo-ghy approved these changes Mar 8, 2024

View reviewed changes

YiwenWu force-pushed the spark_func_regexp branch from ef42d21 to dc4119d Compare March 9, 2024 03:57

JiajunBernoulli reviewed Mar 10, 2024

View reviewed changes

[CALCITE-6278] Add REGEXP, REGEXP_LIKE function (enabled in Spark lib…

78acd2e

…rary)

YiwenWu force-pushed the spark_func_regexp branch from 6a91261 to 78acd2e Compare March 10, 2024 06:08

macroguo-ghy merged commit d8804b4 into apache:main Mar 12, 2024

		@@ -0,0 +1,243 @@
		# spark.iq - Babel test for Spark dialect of SQL

Conversation

YiwenWu commented Mar 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caicancai Mar 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JiajunBernoulli Mar 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caicancai Mar 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YiwenWu Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

julianhyde Mar 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

macroguo-ghy left a comment

Choose a reason for hiding this comment

Uh oh!

jduo commented Mar 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YiwenWu commented Mar 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Mar 10, 2024

Quality Gate passed

Uh oh!

macroguo-ghy commented Mar 11, 2024

Uh oh!

jduo commented Mar 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

YiwenWu commented Mar 3, 2024 •

edited

Loading

caicancai Mar 3, 2024 •

edited

Loading

JiajunBernoulli Mar 3, 2024 •

edited

Loading

caicancai Mar 3, 2024 •

edited

Loading

YiwenWu Mar 5, 2024 •

edited

Loading

julianhyde Mar 5, 2024 •

edited

Loading

jduo commented Mar 9, 2024 •

edited

Loading

YiwenWu commented Mar 10, 2024 •

edited

Loading