Prevent contains-PrefixRange optimization if not preceded by wildcards

[gerashegalov]

Prevent '^[0-9]{n}' from being processed as spark_rapids_jni::literal_range_pattern that currently only supports "contains", not "starts with"

Fixes #10928

Also adding missing tailrec annotations to recursive parser methods.

Signed-off-by: Gera Shegalov [email protected]

[gerashegalov]

build

[thirtiseven]

Oh, i see. ^[0-9]{n} is not a "prefix range" pattern, but ^.*[0-9]{n} is. And stripLeadingWildcards(astLs) will strip the ^ no matter if it is follows a .*. Patterns like ^.*[0-9]{n} won't be rewritten after the fix, but I think it's fine.

Thanks for fixing this!

[thirtiseven]

UT failed, can update to:

  test("regex rewrite contains") {
    import RegexOptimizationType._
    val patterns = Seq(".*abc.*", ".*(abc).*", "^.*(abc).*$", "^.*(.*)(abc).*.*", 
        raw".*\w.*\Z", raw".*..*\Z")
-    val excepted = Seq(Contains("abc"), Contains("abc"), NoOptimization, Contains("abc"), 
+    val excepted = Seq(Contains("abc"), Contains("abc"), NoOptimization, NoOptimization, 
        NoOptimization, NoOptimization)
    verifyRewritePattern(patterns, excepted)
  }

  test("regex rewrite prefix range") {
    import RegexOptimizationType._
    val patterns = Seq(
      "(.*)abc[0-9]{1,3}(.*)",
      "(.*)abc[0-9a-z]{1,3}(.*)",
      "(.*)abc[0-9]{2}.*",
      "^abc[0-9]{1,3}",
      "火花急流[\u4e00-\u9fa5]{1}")
    val excepted = Seq(PrefixRange("abc", 1, 48, 57),
      NoOptimization,
      PrefixRange("abc", 2, 48, 57),
-      PrefixRange("abc", 1, 48, 57),
+      NoOptimization,
      PrefixRange("火花急流", 1, 19968, 40869))
    verifyRewritePattern(patterns, excepted)
  }

[gerashegalov]

Patterns like ^.*[0-9]{n} won't be rewritten after the fix, but I think it's fine.

Thanks for the pointer, I accommodated this case.

[gerashegalov]

build

[NVnavkumar]

LGTM

[thirtiseven]

Added a followup item to support PrefIxRange rewrite startsWith ^ in #10741

[gerashegalov]

Added a followup item to support PrefIxRange rewrite startsWith ^ in #10741

@thirtiseven Just a thought because I am not sure whether it is a high priority for our users: it seems like the current kernel is not hard to generalize for multiple ranges in a regex such as prefix[a-zA-Z0-9]

[thirtiseven]

Added a followup item to support PrefIxRange rewrite startsWith ^ in #10741

@thirtiseven Just a thought because I am not sure whether it is a high priority for our users: it seems like the current kernel is not hard to generalize for multiple ranges in a regex such as prefix[a-zA-Z0-9]

Yes, it's not hard to do, added to the epic issue, thanks! I see some use cases from users, but the priority does not seem very high.

[gerashegalov]

For slightly better efficiency I should probably recurse here with noWildCards but there is no correctness issue. We can fix it in a follow-up PR.