Skip to content

test: move regex coverage to Comet SQL tests and fix fixtures whose patterns lose their backslashes #6627

Open

Description

@andygrove

Part of #6615.

What / Why

This issue moves the regex coverage in CometRegExpJvmSuite, CometRegexParitySuite and the rlike tests in CometExpressionSuite to Comet SQL tests under sql-tests/expressions/string/. Static analysis on main at 3bc2faa found the following:

Suite Covered Convert Blocked Split Keep Total
CometRegExpJvmSuite 12 53 8 0 1 74
CometRegexSuite 0 0 0 0 14 14
CometRegexParitySuite 0 3 0 0 0 3
CometExpressionSuite 2 2 0 0 0 4
Total 14 58 8 0 15 95

Only 14 of the 95 are covered by a fixture today, and none of those depends on \d, \w or a backreference. The reason is the first item below.

Fix first: seven regex fixtures don't run the patterns they name

Spark's SQL string-literal unescaping (unescapeSQLString, with spark.sql.parser.escapedStringLiterals=false) drops the backslash before any character that is not a recognized escape. So '\d' becomes d, '\w' becomes w, '\1' becomes 1 and '\+' becomes +.

These fixtures write single backslashes:

Fixture Lines
regexp_extract.sql 28, 32, 36, 44, 48, 56
regexp_extract_all.sql 28, 32, 36, 44, 52
regexp_instr.sql 28, 32, 40, 48
regexp_replace_java.sql 27, 30, 34, 38, 42, 50
regexp_replace_rust.sql 29, 32, 36
rlike_java.sql 27, 37, 41, 49
rlike_auto_native.sql 38

Examples:

  • rlike_java.sql:27 runs ^d+$.
  • The "backreference (Java-only)" queries run ^(w)1$, which contains no backreference.
  • regexp_replace_java.sql:34 ('(\d+)-(\d+)') matches no row, so its $2-$1 replacement is never applied.
  • rlike_auto_native.sql:38 ('a\+b') is a+b, i.e. one or more a followed by b, not the literal string.

The two *_enabled.sql regex fixtures already write '\\d' correctly. Use '\\d' or a raw literal r'\d'; raw literals work on every supported Spark version.

The Scala tests have the same bug. In CometExpressionSuite's rlike test (L1507), none of the classes and anchors (\d \D \w \W \b \B \h \H \s \S \v \V \A \Z \z) ever reached a regex engine. In rlike whitespace (L1473), six of the 13 patterns are literals that can never match. So the conversion has to fix the patterns, not copy them.

Work

Line numbers refer to CometRegExpJvmSuite unless marked; "parity" means CometRegexParitySuite.

The full per-test add text is in the analysis; this is the condensed version. Use expect_dispatch(rlike) / expect_native(rlike) wherever the Scala test pins the engine. The explain-text checks contains("JVM codegen dispatcher: rlike") map one-to-one onto these. assertCodegenRan is weaker: it passes if anything in the block compiled a kernel.

Existing fixtures to fix and extend

  • rlike_java.sql
    • Covers L65-L155.
    • Fix lines 27, 37, 41 and 49.
    • Add '\\d+' and '^(\\w)\\1$' in a projection and in a filter, 'foo(?=bar)', '(?<=foo)bar', '(?<digit>\\d)', '^$', an all-NULL table, and the backreference filter with GROUP BY.
    • All of these patterns are outside the native subset, so pin them with expect_dispatch(rlike).
  • rlike_auto_native.sql
    • Covers L224 and L606 (4.0+).
    • Line 28 becomes expect_native(rlike). The file currently says routing is asserted in the Scala suites this issue deletes.
  • rlike_rust.sql
    • Covers L501, plus CometExpressionSuite L1473 and L1507.
    • Add expect_native(rlike) for '\\d+' under the opt-in.
    • Add CR, LF and CRLF rows with ^ / $ anchors.
    • Add the curated class, qualifier and anchor set with doubled backslashes.
  • routing_regex_enabled.sql
    • Covers L488 and L943.
    • Add expect_dispatch(rlike) for '\\d+', and expect_dispatch(regexp_instr), which no fixture pins today.
  • routing_regex_disabled.sql
    • Covers L419 and L549.
    • Add expect_fallback(rlike: spark.comet.exec.scalaUDF.codegen.enabled=false) for the 64-branch pattern and for '\\d+'.
  • regexp_extract.sql
    • Covers L682-L722.
    • Fix the backslashes, add rows, and add all-NULL input.
  • regexp_extract_all.sql
    • Covers L730-L757.
    • Fix the backslashes, and add '([a-z])(\\d)' with groups 1 and 2, and 'foo(?=ba[rz])'.
  • regexp_replace_java.sql
    • Covers L766-L797.
    • Fix the backslashes.
    • Add '(\\w+) (\\w+)' with '$2 $1', 'foo(?=bar)', the empty pattern, and all-NULL input.
  • regexp_instr.sql
    • Covers L852-L871.
    • Fix the backslashes, and add groups 1 and 2 and a lookahead.
  • split_java.sql
    • Covers L879, L885 and L909.
    • Add 'x,,y', '\\d+' and all-NULL input.

New fixtures

  • rlike_java_batches.sql: L169, 5000 generated rows across several batches.
  • rlike_routing.sql
    • Covers the analyzer-budget tests: L237-L465 and L531.
    • Write each pattern end to end as expect_dispatch(rlike) or expect_native(rlike).
    • Patterns up to 16,388 characters must be spelled out in full; repeat() would make the pattern non-literal and change the route.
  • rlike_native_parity.sql
    • Covers parity L129 and L136.
    • 43 subjects × 30 corpus patterns, in a projection and in a filter, all expect_native(rlike).
    • Write control characters and line terminators as \uXXXX.
  • rlike_native_batches.sql: parity L163, with spark.comet.batchSize=64 and 5000 rows.
  • rlike_collation.sql and rlike_collation_opt_in.sql (MinSparkVersion: 4.0)
    • L620 and L648 use a collated subject with expect_dispatch(rlike).
    • RLike is not in Spark's collation-coercion list, so the pattern stays a plain literal.
  • regexp_extract_batches.sql (L917) and regexp_combined.sql (L929).
  • regexp_replace_errors.sql and regexp_replace_errors_row_filter.sql (4.0+)

For the invalid-pattern tests (L144, L272, L579), check the expect_error substring on each profile. Spark's codegen path throws a raw PatternSyntaxException ("Unclosed character class") from generated init code, while the interpreted path wraps it in INVALID_PARAMETER_VALUE.PATTERN.

Blocked

Stays in Scala

  • All 14 CometRegexSuite tests. They are unit tests of CometRegex.supportLevel, the plan-time analyzer, with no Spark session. Several of the patterns don't compile in Java at all.
  • L585. It pins a Comet-only failure: the opt-in sends a lookahead to the Rust engine, which rejects it, while Spark succeeds. expect_error needs both engines to fail.

Notes on the Scala tests

  • L144 ("invalid pattern falls back to Spark") never checks a fallback. It is a weaker duplicate of L579.
  • L943 ("always run via the JVM dispatcher") never pins the dispatcher. Its queries duplicate L682, L730 and L852.
  • Covered by other tests: L606 is a subset of L224, and parity L129 is implied by L136.

Done when

  • Every test in the table below is accounted for. Either a fixture covers it (the same inputs, configs and assertion strength, or stronger), the fixture that already covers it is cited, or it stays in Scala for the reason given above.
  • Each converted Scala test is deleted in the PR that adds its coverage. A suite left empty is deleted and removed from the suite lists in .github/workflows/pr_build_linux.yml and pr_build_macos.yml.
  • The new and changed fixtures pass on every Spark profile. Apply run-all-spark-profiles, because pull requests run only the default profile.
Every Scala test in scope, with its verdict and target fixture
Suite Line Test Verdict Target
CometExpressionSuite 1367 rlike simple case covered string/rlike_rust.sql
CometExpressionSuite 1462 rlike with non-scalar pattern runs via codegen dispatcher covered string/routing_regex_enabled.sql
CometExpressionSuite 1473 rlike whitespace convert string/rlike_rust.sql
CometExpressionSuite 1507 rlike convert string/rlike_rust.sql
CometRegExpJvmSuite 65 rlike: projection produces Java regex semantics with null handling convert string/rlike_java.sql
CometRegExpJvmSuite 72 rlike: predicate filters rows using Java regex semantics convert string/rlike_java.sql
CometRegExpJvmSuite 79 rlike: backreference in projection (Java-only construct) convert string/rlike_java.sql
CometRegExpJvmSuite 90 rlike: backreference in predicate (Java-only construct) convert string/rlike_java.sql
CometRegExpJvmSuite 101 rlike: lookahead pattern (Java-only construct) convert string/rlike_java.sql
CometRegExpJvmSuite 108 rlike: lookbehind pattern (Java-only construct) convert string/rlike_java.sql
CometRegExpJvmSuite 114 rlike: embedded case-insensitive flag (Java-only construct) covered string/rlike_java.sql
CometRegExpJvmSuite 120 rlike: named groups (Java-only construct) convert string/rlike_java.sql
CometRegExpJvmSuite 126 rlike: empty pattern matches every non-null row covered string/rlike_java.sql
CometRegExpJvmSuite 132 rlike: empty subject string is handled correctly convert string/rlike_java.sql
CometRegExpJvmSuite 138 rlike: all-null subject column produces all-null result convert string/rlike_java.sql
CometRegExpJvmSuite 144 rlike: invalid pattern falls back to Spark convert string/rlike_java.sql
CometRegExpJvmSuite 155 rlike: combines with filter, projection, and aggregate convert string/rlike_java.sql
CometRegExpJvmSuite 169 rlike: many rows spanning multiple batches convert string/rlike_java_batches.sql (new)
CometRegExpJvmSuite 224 rlike: safe literal pattern takes the native path by default convert string/rlike_auto_native.sql
CometRegExpJvmSuite 237 rlike: Rust class set operations stay on the dispatcher convert string/rlike_routing.sql (new)
CometRegExpJvmSuite 258 rlike: leading-bracket class ranges stay on the dispatcher convert string/rlike_routing.sql (new)
CometRegExpJvmSuite 272 rlike: raw [ range endpoint stays on the dispatcher and preserves Spark error blocked: #6617 string/rlike_routing.sql (new)
CometRegExpJvmSuite 287 rlike: over-budget counted repetition stays on the dispatcher convert string/rlike_routing.sql (new)
CometRegExpJvmSuite 329 rlike: compile-budget boundary stays native convert string/rlike_routing.sql (new)
CometRegExpJvmSuite 342 rlike: exact-zero counted repetitions stay native convert string/rlike_routing.sql (new)
CometRegExpJvmSuite 356 rlike: aggregate expansion budget stays on the dispatcher convert string/rlike_routing.sql (new)
CometRegExpJvmSuite 370 rlike: nested quantified stars stay on the dispatcher convert string/rlike_routing.sql (new)
CometRegExpJvmSuite 384 rlike: capturing groups copied by counted repetition stay on the dispatcher convert string/rlike_routing.sql (new)
CometRegExpJvmSuite 406 rlike: uncounted capturing groups stay on the dispatcher convert string/rlike_routing.sql (new)
CometRegExpJvmSuite 419 rlike: uncounted capturing groups fall back to Spark when the dispatcher is disabled convert string/routing_regex_disabled.sql
CometRegExpJvmSuite 440 rlike: full uncounted-capture reproducer falls back to Spark when JVM dispatcher is disabled blocked: other string/routing_regex_disabled_large.sql (new)
CometRegExpJvmSuite 465 rlike: repeated complex character classes stay on the dispatcher convert string/rlike_routing.sql (new)
CometRegExpJvmSuite 488 rlike: unsafe literal pattern stays on the JVM dispatcher by default convert string/routing_regex_enabled.sql
CometRegExpJvmSuite 501 rlike: unsafe but Rust-accepted pattern is native after opt-in convert string/rlike_rust.sql
CometRegExpJvmSuite 516 rlike: non-literal pattern stays on the dispatcher covered string/routing_regex_enabled.sql
CometRegExpJvmSuite 531 rlike: null literal pattern stays on the dispatcher blocked: #6618 string/rlike_routing.sql (new)
CometRegExpJvmSuite 549 rlike: unsafe pattern falls back to Spark when the dispatcher is disabled convert string/routing_regex_disabled.sql
CometRegExpJvmSuite 559 rlike: in-subset pattern stays native when the dispatcher is disabled covered string/routing_regex_disabled.sql
CometRegExpJvmSuite 579 rlike: invalid pattern preserves Spark exception type and message blocked: #6617 string/rlike_java.sql
CometRegExpJvmSuite 585 rlike: Java-only pattern with allowIncompatible keeps existing opt-in behavior keep
CometRegExpJvmSuite 606 rlike: UTF8_BINARY safe literal is native on Spark 4 convert string/rlike_auto_native.sql
CometRegExpJvmSuite 620 rlike: non-default collation on the subject stays on the dispatcher convert string/rlike_collation.sql (new)
CometRegExpJvmSuite 634 rlike: non-default collation on the pattern stays on the dispatcher blocked: #6618 string/rlike_collation.sql (new)
CometRegExpJvmSuite 648 rlike: collated subject stays on dispatcher even with allowIncompatible convert string/rlike_collation_opt_in.sql (new)
CometRegExpJvmSuite 664 rlike: collated pattern stays on dispatcher even with allowIncompatible blocked: #6618 string/rlike_collation_opt_in.sql (new)
CometRegExpJvmSuite 682 regexp_extract: basic group extraction convert string/regexp_extract.sql
CometRegExpJvmSuite 691 regexp_extract: group 0 returns entire match convert string/regexp_extract.sql
CometRegExpJvmSuite 697 regexp_extract: no match returns empty string covered string/regexp_extract.sql
CometRegExpJvmSuite 703 regexp_extract: backreference pattern (Java-only) convert string/regexp_extract.sql
CometRegExpJvmSuite 709 regexp_extract: lookahead pattern (Java-only) convert string/regexp_extract.sql
CometRegExpJvmSuite 715 regexp_extract: embedded flags (Java-only) covered string/regexp_extract.sql
CometRegExpJvmSuite 722 regexp_extract: all-null column convert string/regexp_extract.sql
CometRegExpJvmSuite 730 regexp_extract_all: basic extraction of all matches convert string/regexp_extract_all.sql
CometRegExpJvmSuite 736 regexp_extract_all: group 0 returns full matches covered string/regexp_extract_all.sql
CometRegExpJvmSuite 742 regexp_extract_all: multiple groups convert string/regexp_extract_all.sql
CometRegExpJvmSuite 751 regexp_extract_all: no matches returns empty array covered string/regexp_extract_all.sql
CometRegExpJvmSuite 757 regexp_extract_all: lookahead pattern (Java-only) convert string/regexp_extract_all.sql
CometRegExpJvmSuite 766 regexp_replace: basic replacement convert string/regexp_replace_java.sql
CometRegExpJvmSuite 772 regexp_replace: backreference in pattern (Java-only) convert string/regexp_replace_java.sql
CometRegExpJvmSuite 778 regexp_replace: backreference in replacement convert string/regexp_replace_java.sql
CometRegExpJvmSuite 785 regexp_replace: lookahead pattern (Java-only) convert string/regexp_replace_java.sql
CometRegExpJvmSuite 791 regexp_replace: empty pattern replaces between characters convert string/regexp_replace_java.sql
CometRegExpJvmSuite 797 regexp_replace: all-null column convert string/regexp_replace_java.sql
CometRegExpJvmSuite 803 regexp_replace: invalid group reference surfaces the original Spark exception blocked: #6617 string/regexp_replace_errors.sql (new)
CometRegExpJvmSuite 828 regexp_replace: invalid group reference in a filter surfaces the original exception blocked: #6617 string/regexp_replace_errors_row_filter.sql (new)
CometRegExpJvmSuite 852 regexp_instr: basic position finding convert string/regexp_instr.sql
CometRegExpJvmSuite 858 regexp_instr: specific group position convert string/regexp_instr.sql
CometRegExpJvmSuite 865 regexp_instr: no match returns 0 covered string/regexp_instr.sql
CometRegExpJvmSuite 871 regexp_instr: lookahead (Java-only) convert string/regexp_instr.sql
CometRegExpJvmSuite 879 split: basic regex split convert string/split_java.sql
CometRegExpJvmSuite 885 split: regex pattern convert string/split_java.sql
CometRegExpJvmSuite 891 split: with limit covered string/split_java.sql
CometRegExpJvmSuite 897 split: limit -1 returns all covered string/split_java.sql
CometRegExpJvmSuite 903 split: lookahead pattern (Java-only) covered string/split_java.sql
CometRegExpJvmSuite 909 split: all-null column convert string/split_java.sql
CometRegExpJvmSuite 917 regexp_extract: many rows spanning multiple batches convert string/regexp_extract_batches.sql (new)
CometRegExpJvmSuite 929 all regexp expressions combined in one query convert string/regexp_combined.sql (new)
CometRegExpJvmSuite 943 expressions with no native path always run via the JVM dispatcher convert string/routing_regex_enabled.sql
CometRegexParitySuite 129 analyzer admits every corpus pattern convert string/rlike_native_parity.sql (new)
CometRegexParitySuite 136 native rlike matches Spark for every admitted pattern convert string/rlike_native_parity.sql (new)
CometRegexParitySuite 163 native rlike matches Spark across multiple Arrow batches convert string/rlike_native_batches.sql (new)
CometRegexSuite 38 admits ASCII literals, classes, quantifiers, groups, and alternation keep
CometRegexSuite 69 admits lexer-boundary patterns that a substring search would misclassify keep
CometRegexSuite 78 rejects constructs that diverge from Java regex or are unrecognized keep
CometRegexSuite 110 rejects scanner edge cases the whitelist must not silently admit keep
CometRegexSuite 131 rejects Rust-only character-class set operations keep
CometRegexSuite 135 rejects unescaped ] used as a character-class atom or range endpoint keep
CometRegexSuite 141 rejects unescaped [ used as a character-class range endpoint keep
CometRegexSuite 146 rejects counted or nested patterns that can exceed the Rust compile budget keep
CometRegexSuite 163 rejects aggregate patterns that exceed the Rust compile budget keep
CometRegexSuite 179 rejects nested quantified patterns that can exceed the Rust compile budget keep
CometRegexSuite 192 rejects capturing groups duplicated by counted repetition past the compile budget keep
CometRegexSuite 208 rejects repeated complex character classes that can exceed the Rust compile budget keep
CometRegexSuite 222 rejects uncounted capturing groups that exceed the compile budget keep
CometRegexSuite 230 capture cost propagates through uncounted repetition and composition keep
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions