Skip to content

feat: run rlike natively by default for Java-equivalent literal patterns - #5415

Open
sam-1112 wants to merge 5 commits into
apache:mainfrom
sam-1112:rlike-native-compatible-subset-5351
Open

feat: run rlike natively by default for Java-equivalent literal patterns#5415
sam-1112 wants to merge 5 commits into
apache:mainfrom
sam-1112:rlike-native-compatible-subset-5351

Conversation

@sam-1112

@sam-1112 sam-1112 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #5351.

Rationale for this change

rlike already has a native kernel, but the Rust regex implementation is treated as potentially incompatible with Java regex and is therefore opt-in. Before this PR, literal patterns ran through the JVM codegen dispatcher by default unless spark.comet.expression.RLike.allowIncompatible=true.

#4310 correctly concluded that the Rust engine cannot be made fully compatible with Java regex, so this PR does not change the engine globally. Instead, it introduces a deliberately restricted, conservative whitelist for plan-time UTF8_BINARY literal patterns.

Patterns admitted by the whitelist use native execution by default and are covered by Java-versus-Rust differential tests. Out-of-subset non-null literals preserve the existing dispatcher / allowIncompatible behavior. Non-literal and NULL patterns use the dispatcher; NULL is never considered a usable native pattern.

What changes are included in this PR?

  • Add CometRegex, a recursive-descent parser implementing a conservative whitelist rather than relying on substring matching. Unrecognized constructs are Incompatible.
    • Admitted: printable ASCII literals, simple ASCII classes, greedy * + ? {n} {n,} {n,m}, capturing and non-capturing groups ((?:…)), alternation, and escaped metacharacters, provided counted and nested forms remain within conservative native compile-size and depth budgets.
    • Rejected: ^ $ . \d \w \s, lookaround, backrefs, possessive/lazy quantifiers, inline flags, \p / \u / \0, nested classes, Rust-only class set operators (&&, ~~, --), unescaped ] class atoms or range endpoints, raw [ range endpoints, counted or nested patterns exceeding the conservative compile budget, non-ASCII patterns, and non-default Spark 4 collation on either operand.
    • Compile-budget gates: individual counted bounds above 256, aggregate estimated expansion above 4096, and group nesting deeper than 32 remain on the JVM dispatcher. For unbounded {n,}, a lower bound of zero still retains the inner expression's compilation cost; exact-zero {0} and {0,0} repetitions remain distinct.
  • CometRLike consults the analyzer. In-subset literals become Compatible() with no nativeOptIn hint and convert to native without opt-in.
  • Out-of-subset non-null literals stay Compatible(nativeOptIn = …) and convert to the dispatcher unless allowIncompatible=true.
  • Non-literal and NULL patterns always convert to the dispatcher (literalPattern only matches a non-null UTF8String literal).
  • allowIncompatible still forces native execution for any non-null literal.
  • Update compatibility/regex.md, expressions.md (rlike / regexp / regexp_like), and the rlike expression-audit note.
  • This first PR covers rlike only. It does not add pattern rewriting such as (?-u), propagate collation into the native kernel, or change regexp_replace, split, regexp_extract, or regexp_extract_all.

A conservative whitelist identifies a deliberately restricted subset for which Java and Rust find semantics are expected to agree and are covered by differential parity tests. It is not a formal proof of equivalence.

How are these changes tested?

  • CometRegexSuite: admit / reject coverage, including:
    • lexer-boundary cases such as [(?=], \\d, and [.];
    • scanner edges such as \A, \Z, {2,1}, and an unclosed (;
    • Rust-only character-class set operators;
    • leading ] and raw [ class-range boundaries;
    • counted-expansion limits and nested-group depth;
    • aggregate compile-budget accounting;
    • rejection of the {0,} regression (([^;]{256}){0,}){256};
    • continued admission of the exact-zero {0} and {0,0} controls.
  • CometRegexParitySuite: every pattern in the admitted corpus × ASCII / non-ASCII / newline / NULL subjects, with native results compared against java.util.regex.Pattern.find. The suite also asserts that EXPLAIN output does not contain JVM codegen dispatcher: rlike. The multi-batch corpus contains 5000 rows with batch size 64. NULL subjects match Spark; a NULL pattern is not part of this native corpus.
  • CometRegExpJvmSuite: routing coverage for:
    • native default and dispatcher default;
    • explicit incompatible opt-in;
    • non-literal and NULL patterns;
    • dispatcher-disabled native execution and fallback;
    • invalid regex handling;
    • Java-only patterns with allowIncompatible=true, including expected native compilation failures for unsupported Rust constructs;
    • Spark 4 collation on the subject and pattern;
    • Rust-only class operations and Java/Rust class-boundary differences;
    • patterns exceeding native compile limits.
  • The {0,} regression is verified against Spark using non-foldable input, including NULL, and remains on the JVM dispatcher.
  • SQL file rlike_auto_native.sql verifies default-configuration result equality.
  • Validated locally with Spark 3.5.9 and 4.1.3 on JDK 17.
  • After the review follow-up, the targeted CometRegexSuite and CometRegExpJvmSuite completed on Spark 4.1 / Scala 2.13:
Tests: succeeded 73, failed 0
BUILD SUCCESS

Other supported Spark and Scala profiles are covered by CI.

Benchmark

CometRegExpBenchmark, 1,048,576 rows, Apple M4, JDK 17.

Native vs Spark for in-subset patterns

After this PR, these patterns use native execution by default.

Pattern Spark Native Speedup vs Spark
[0-9]+ 397 ms 84 ms 4.7X
abc|def|ghi 2304 ms 80 ms 28.8X
[a-zA-Z][0-9]+ 1075 ms 125 ms 8.6X
(ab){2,} 1114 ms 77 ms 14.5X

Default Comet execution before and after this PR

These measurements use identical in-subset queries. Both modes were measured on the PR base commit in the same run: the JVM dispatcher represents the pre-PR default, while allowIncompatible=true selects the same native kernel that this PR now chooses automatically.

Pattern Base: JVM dispatcher Native Speedup vs dispatcher
[0-9]+ 384 ms 84 ms 4.6X
abc|def|ghi 2303 ms 80 ms 28.8X
[a-zA-Z][0-9]+ 1070 ms 125 ms 8.6X
(ab){2,} 1091 ms 77 ms 14.2X

On this ASCII REPEAT workload, the pre-PR dispatcher is essentially Spark-cost. Switching the in-subset default from the dispatcher to native execution is the performance improvement that matters for this PR.

Out-of-subset control

\d+ remains outside the automatic-native subset because Java and Rust differ in their Unicode digit semantics. It uses the JVM dispatcher by default; allowIncompatible=true explicitly selects the native path.

The benchmark SQL preserves the backslash so Spark receives \d+ rather than parsing it as d+.

Pattern Spark Default Comet: JVM dispatcher Opt-in native Native speedup vs dispatcher
\d+ 373 ms 365 ms 77 ms 4.7X

The corrected JVM-dispatcher measurement is close to Spark cost, as expected. The explicitly opted-in native path is approximately 4.7X faster than the dispatcher on this ASCII workload.

The native implementation matches over Arrow buffers and is expected to avoid the dispatcher's per-row toString() / Matcher allocations. This allocation difference was not measured with JFR or a GC log in this PR.

Add a plan-time whitelist so UTF8_BINARY literals the analyzer can prove
equivalent to Java regex take the native path without allowIncompatible.
Out-of-subset literals stay on the JVM dispatcher.

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Reviewed the complete 13-file diff from 367ca64be4e62dd10dc4c932de9ca738355a45fa to 86729123819387bbfa132c5c81956f82ea60b193, including the live discussion and CI state. Five independent full-diff passes were followed by independent source and executable verification of the retained cases.

The intended optimization is narrowly scoped to default-native RLike literals. Three pattern-specific P2 regressions remain: Rust character-class set operations change results, leading-] ranges independently change results, and Java-valid patterns can be admitted despite exceeding Rust's compilation limits. Each is detailed inline; none requires opting into incompatible execution.

Prior state and problem

The native RLike kernel already existed, but Java and Rust regex syntax and semantics differ. At the base revision, ordinary literal patterns used Spark's own generated regex implementation through the JVM dispatcher by default; native execution required spark.comet.expression.RLike.allowIncompatible=true. If the dispatcher was disabled, the default expression could fall back to Spark.

This preserves Java behavior but leaves JNI/dispatcher and per-row Java regex costs on patterns that both engines can evaluate equivalently. The PR aims to remove those costs for an automatically recognized subset while keeping other regex expressions and explicitly incompatible execution behavior separate.

Design approach

CometRegex introduces a recursive-descent admission parser rather than a substring-based filter. It recognizes printable ASCII literals, selected character classes, ordinary greedy quantifiers, capturing/non-capturing groups, and alternation. Unrecognized syntax is rejected from automatic native selection; anchors, wildcard dot, shorthand classes, lookaround, backreferences, and flags are intentionally outside the subset.

CometRLike additionally requires a non-null literal and default string collation on both operands for automatic selection. The existing native kernel and Cargo lock are unchanged, and the pattern is serialized without rewriting. Consequently, the admission decision must establish both equivalent Boolean matching and native compilability.

Correctness / compatibility analysis

The retained cases violate two different parts of that admission contract:

  • [a~~b] and [a-z--b] are admitted, but Rust interprets class set operations that Java does not interpret the same way. Concrete non-null inputs produce opposite Booleans.
  • []-a] is also admitted, without either set operator. Java treats the initial ] as a range endpoint; Rust treats this spelling differently. Both positive and negated classes can add or remove matching rows.
  • [^;]{20000}, a{1000000}, nested counted repetitions, and sufficiently nested groups are admitted even though default Rust compilation rejects them. Native plan construction propagates that rejection instead of returning to the dispatcher.

Verification used the unmodified pinned scanner, the exact regex 1.13.1 dependency graph with archive/source checksums matched to Cargo.lock, and real Spark 3.5.9 and 4.0.4 expressions with non-foldable input. The retained cases were checked in both interpreted evaluation and forced generated projections; NULL-subject controls were also checked. These are component/runtime probes plus source-traced integration, not a claimed full Comet/JNI query run.

The unchanged four-test CometRegexSuite passed in isolated builds on Scala 2.12.18 and 2.13.17, and suite-registration and diff-whitespace checks passed. The existing examples do not cover the counterexamples above. CI started during this review: at the 17:28 UTC refresh, CodeQL and Linux/macOS lint checks had succeeded, while native/JVM builds, Rust tests, Java lint, and the benchmark check were still running. No completed full integration-suite result is claimed.

Key design decisions

  • Keep automatic admission distinct from the existing incompatible opt-in. The latter remains an explicit request to use the Rust implementation for applicable literals, not an equivalence guarantee.
  • Check both operand collations for the automatic route. Spark 3.x uses the existing no-collation shim; Spark 4.x uses the existing collation-aware type checks. Explicit opt-in can bypass the automatic equivalence gate.
  • Exclude NULL and non-literal patterns from native applicability. They continue through the dispatcher, or fall back when the dispatcher is unavailable.
  • Preserve the generic expression-enabled gate. Disabling RLike itself still prevents this serializer from converting it; disabling only the dispatcher no longer disables eligible native RLike expressions.

Implementation sketch

The serializer extracts a UTF8String literal, checks the operand types, and calls CometRegex.supportLevel. An admitted pattern becomes Compatible() without a native-opt-in hint. Conversion then emits the existing RLike protobuf through createBinaryExpr; the native builder passes the unchanged literal to RLike::try_new, which uses Regex::new and is_match.

Other applicable literals retain their opt-in hint and default JVM dispatcher route. The patch also adds analyzer and parity suites, expands routing/NULL/collation tests, registers the new suites in both OS workflows, adds SQL cases, and updates documentation and benchmark mode selection. No native engine or dependency change accompanies the newly automatic route.

Behavioral changes worth calling out

For admitted literals, default Comet execution now uses the native engine even with the dispatcher disabled. That is the intended performance change, but it also makes any admission mistake observable without a user accepting regex incompatibility.

Out-of-subset literals continue to use the JVM by default, and non-literal/NULL patterns do not become native. Other regex functions retain their existing routing rules. The compile-limit cases are a change from successful default JVM evaluation to native construction failure; the two character-class cases are silent result changes, not merely different match spans or capture groups.

Suggested improvements

Tighten class admission for both Rust-only set operators and the independent leading-] range boundary. Add positive and negated differential examples, including subjects that distinguish literal operator characters and range interiors, and assert that rejected patterns retain JVM routing by default.

Make native compilability part of the admission contract, with conservative aggregate-expansion/depth bounds or a reliable validation/fallback path. A per-integer bound or source-length limit alone does not cover nested repetition expansion. Add the concrete size/depth cases to routing and error-behavior regression coverage, then verify the full parity/routing suites on the supported Spark profiles as CI completes.

Comment on lines +223 to +225
if (startsWith("&&") || peek == '[') {
return false
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Exclude Rust-only class operators from automatic native routing

This gate rejects && but admits ~~ and subtraction forms such as [a-z--b]. The pinned analyzer returns Compatible for [a~~b]; Spark 3.5.9/4.0.4 return true on subject ~, while the locked Rust regex 1.13.1 returns false because ~~ is symmetric difference. Likewise, [a-z--b] matches b in Spark but not Rust. I verified the Spark results in both interpreted and generated evaluation with a non-foldable input. Since CometRLike.convert now sends these literals to the unchanged native matcher without allowIncompatible=true, existing projections and filters silently change results. Keep these class forms outside automatic admission and add differential/routing regressions before selecting native for them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — the first revision only rejected &&, so [a~~b] / [a-z--b] were wrongly admitted and would have taken the native path without allowIncompatible.

7e51a22 now treats && / ~~ / -- inside a character class as incompatible. Those literals stay on the JVM dispatcher by default. Single ~ in a class (e.g. [a~b]) is still admitted; it is a Java literal, not a Rust set operator.

Regressions:

  • CometRegexSuite: analyzer returns Incompatible for [a~~b], [^a~~b], [a-z--b], [^a-z--b], [a&&b]
  • CometRegExpJvmSuite: EXPLAIN still shows the dispatcher, and checkSparkAnswerAndOperator matches Spark on a non-foldable column (~, a, b, x, and b/a/z/- for the subtraction form), including NULL

No runtime compile-fallback; these stay off automatic native.

Comment on lines +226 to +230
val ranging = lastAtom.isDefined && peek == '-' && peekOffset(1).exists(_ != ']')
if (ranging) {
consume() // '-'
parseClassAtom() match {
case Some(end) if end >= lastAtom.get =>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Reject ranges starting at an unescaped leading closing bracket

The initial literal ] is stored in lastAtom and then accepted as a range endpoint. For the admitted pattern []-a], Java/Spark interpret the range from ] through a, but Rust treats this spelling as the literals ], -, and a. On both Spark 3.5.9 and 4.0.4, interpreted and generated evaluation return true for _ and false for -; the locked native regex returns the opposite results. The negated form also differs. This default-native wrong-answer case contains neither ~~ nor --, so rejecting set operators alone will not fix it. Reject this class-boundary form, or normalize it only after establishing equivalent semantics, and cover both positive and negated variants.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — this is a separate Java/Rust split, not covered by rejecting ~~ / --.

Java treats a leading ] as a literal (so []-a] is the range ]a), while Rust reads it as the closer and then the literals ], -, a. Same mismatch on the negated form. That’s why _ vs - flip between Spark and native.

7e51a22 now never treats an unescaped ] as a class atom or range endpoint, so []-a] and [^]-a] stay off automatic native. Escaped ] ([\\]]) is still admitted.

Tests:

  • analyzer: Incompatible for []-a], [^]-a], []], [^]]
  • routing: dispatcher + Spark-equal answers on _, -, ], a, z, and NULL

Comment on lines +50 to +52
val scanner = new Scanner(pattern)
if (scanner.parseExpr() && !scanner.remaining) {
Compatible()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve JVM routing for patterns beyond native compile limits

Successful syntax scanning does not ensure Regex::new can compile the pattern. For example, this analyzer admits [^;]{20000} and a{1000000}; real Spark 3.5.9/4.0.4 evaluate them on a column containing a as false, but the locked Rust engine rejects both with Compiled regex exceeds size limit of 10485760 bytes. Nested counted repetitions have the same problem, and 251 nested groups exceed Rust's separate depth limit while succeeding in Spark. The unchanged RLike::try_new and native builder propagate these errors from plan creation, with no dispatcher retry, so previously successful default-config queries now fail without opt-in. Require conservative native size/depth applicability or a reliable compilation-validation/fallback path before returning Compatible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep — scanning the syntax isn’t the same as “Rust can compile this.” Spark is happy with [^;]{20000} / a{1000000} / nested {n} / deep groups; the locked regex crate then dies at plan time (Compiled regex exceeds size limit / nest depth) and we have no dispatcher retry. That’s a default-config regression.

Didn’t add a compile-then-fallback path here. 7e51a22 just refuses those at plan time:

  • {n} bigger than 256
  • nested counted product over 4096 ((a{100}){100})
  • more than 32 nested groups

They stay on the JVM dispatcher. a{256} and 32-deep groups still go native. Analyzer + EXPLAIN/Spark-equal tests cover the cases you listed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] {0,} still bypasses the compile-size budget

Rechecked at 0f98989095dee698db71b366998e112bdd1e15a6. The aggregate checks address the earlier sibling cases, but (([^;]{256}){0,}){256} is still classified as Compatible. The {0,} branch returns the lower bound 0, so multiplyWithinBudget reduces the inner cost from 256 to 1. Unlike {0} or {0,0}, this is unbounded repetition and the inner expression still has to be compiled.

I reproduced this with the unmodified current scanner on Scala 2.12 and 2.13. Spark 3.5.9 and 4.0.4 both return true for a non-foldable subject containing a, in interpreted evaluation and forced generated projections. The exact locked Rust regex 1.13.1 instead rejects the pattern with Compiled regex exceeds size limit of 10485760 bytes. The equivalent (([^;]{256})*){256} is correctly rejected by the scanner.

Because the admitted form now selects native execution without opt-in and the native builder propagates the compilation error, this still changes a successful default-config query into a failure. Could we retain the inner compilation cost for unbounded {0,} separately from exact-zero repetitions and add this case to the dispatcher-routing regressions? These checks used real Spark expressions and the locked regex engine plus source-traced native routing, not a full Comet/JNI query run.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that was a real gap. {0,} was using the lower bound 0 as the expansion multiplier, so multiplyWithinBudget collapsed the inner cost to 1 and admitted (([^;]{256}){0,}){256}. That pattern is Java-valid but rejected by the locked Rust regex crate, so the native path would fail a query that Spark currently succeeds.

The unbounded {n,} branch now uses math.max(1, n), so {0,} keeps the inner compile cost. {0} and {0,0} stay exact-zero and are still Compatible.

Coverage:

  • analyzer: (([^;]{256}){0,}){256} is Incompatible; {0} / {0,0} remain Compatible
  • routing: the unbounded pattern stays on the JVM codegen dispatcher and matches Spark
  • exact-zero counterparts stay native and match Spark

Pushed in c62b529a7.

…e budget

Reject Rust-only character-class operators and unescaped ] range endpoints
so incompatible literals stay on the JVM dispatcher. Cap nested counted
expansion and group depth so Java-valid patterns that Rust cannot compile
do not fail native plan construction.
}
if (peek == '\\') {
parseEscape(inClass = true)
} else if (isPrintableAscii(peek)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve Spark errors for raw [ range endpoints

The [ guard in parseClass runs before the range branch consumes -, so this helper can still accept an unescaped [ as the range endpoint. The exact scanner returns Compatible for [@-[] (and [^@-[]). With a non-foldable string input, real Spark 3.5.9 and 4.0.4 reject these patterns: interpreted RLIKE raises an invalid-pattern error, and forced generated-projection construction raises PatternSyntaxException for an unclosed character class. The locked Rust regex 1.13.1 instead compiles [@-[] and matches A. Because CometRLike.convert now trusts this admission result without incompatible opt-in, a malformed-pattern failure becomes a successful predicate under the default native route. Please reject raw [ in the range-endpoint helper as well; the escaped [@-\[] control is valid and equivalent in both engines.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — the class-loop [ guard runs before - is consumed, so it never saw [@-[] / [^@-[] as a nested class. Java rejects those as an unclosed class; Rust compiles them and matches A. That’s a default-native “error becomes true/false” case. This commit rejects an unescaped [ in parseClassAtom as well, so it cannot be a range endpoint. Escaped [@-\[] is still admitted and covered by Java/native parity.
Tests:

  • analyzer: Incompatible for [@-[] / [^@-[]; Compatible for [@-\[]
  • routing: dispatcher + Spark regex error on a non-foldable column (@, [, A, NULL)

Reject unescaped '[' as a character-class range endpoint so malformed
Java patterns like [@-[] stay on the JVM dispatcher and preserve Spark
regex errors. Apply MaxExpansion across concatenation, alternation, and
counted repetition, including zero-count terms, so sibling groups and
repeated atoms cannot bypass compile-size admission.
}
}
outOfSubsetPatterns.foreach { p =>
val query = s"select c1 rlike '${p.pattern}' from parquetV1Table"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve the backslash in the out-of-subset benchmark SQL

With the default spark.sql.parser.escapedStringLiterals=false, interpolating the new \d+ pattern here makes Spark parse it as d+ (reproduced with Spark 3.5.9 and 4.0.4). CometRegex admits d+, so the case labelled Comet (Exec, JVM regex) selects the automatic native branch just like the opted-in case. The generated numeric strings also match none of the rows instead of every row. This leaves the new four-way comparison without a JVM-dispatcher measurement. Please preserve the backslash with an escaped or raw SQL literal, as the parity suite already does, before passing this query to runOutOfSubsetModes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in c62b529a7

The out-of-subset pattern is now escaped before being interpolated into the SQL query:

val escapedPattern = p.pattern.replace("\\", "\\\\")
val query = s"select c1 rlike '$escapedPattern' from parquetV1Table"

sunchao and others added 2 commits August 23, 2026 12:56
Keep `{0,}` from collapsing inner cost so nested patterns like
`(([^;]{256}){0,}){256}` stay on the JVM dispatcher. Escape
backslashes in the rlike benchmark SQL so `\d+` is not parsed as `d+`.
@andygrove

Copy link
Copy Markdown
Member

Note on this review: this was generated by an LLM (Claude Code) at my request while I worked through a review backlog. I have not verified the individual findings myself. Please treat everything below as suggestions to evaluate rather than as authoritative review feedback, and push back on anything that is wrong or already handled.

This is impressive work. A recursive-descent parser implementing an explicit whitelist is much better than the substring matching this replaces, the rejected list reads like someone actually enumerated where Java and Rust diverge rather than guessing, and CometRegexParitySuite comparing against java.util.regex.Pattern.find over the full admitted corpus is the right shape of test. Being explicit that this is "not a formal proof of equivalence" is honest and appreciated.

Four things.

The whitelist needs fuzzing, not just a corpus

The safety property here is: every pattern the parser admits behaves identically in Rust and Java. A fixed corpus tests the patterns you thought of. The bug that matters is the one where the parser admits something you did not think of, and then the result is silently wrong rather than an error.

Would you add a generative test: produce random patterns from a grammar (including plenty that should be rejected), run them through CometRegex, and for every admitted pattern compare Java's find against the Rust kernel over a set of random subjects including non-ASCII, newlines, and empty strings? A few thousand iterations of that would find parser gaps that no hand-written corpus will. It could run as a nightly rather than on every PR if it is slow.

How much of the real world does the subset cover?

., ^, $, \d, \w, and \s are all rejected, and between them they appear in most regexes anyone actually writes. So it would be useful to know what fraction of realistic patterns the whitelist admits. Even a rough number from a corpus you have to hand, or from the regex-heavy TPC-DS queries, would tell us whether 360 lines of parser is buying meaningful acceleration or mostly covering patterns that are already cheap.

That is not an argument against the PR. It is an argument for putting the number in the description, because it determines whether the follow-up work ((?-u) rewriting to admit . and the classes) is urgent or not.

Where do 256, 4096, and 32 come from?

The compile-budget gates are three magic numbers. regex's own default size limit is 10 MB of compiled program, so presumably these were chosen to stay well under it, but the derivation is not written down. Could the constants carry a comment explaining what each one bounds and roughly what compiled size it corresponds to? Otherwise the next person who wants to raise one has no basis for deciding whether it is safe.

Coupling to the regex crate version

The whitelist encodes assumptions about how the regex crate behaves today. A future regex bump could change the semantics of an admitted pattern, and nothing in the build would notice. CometRegexParitySuite running in both PR workflows is good, but does the parity suite actually exercise the Rust kernel through native execution, or does it compare Java against expected values? If the former, a regex upgrade would break it, which is the behavior we want. If the latter, it would be worth adding at least one test that would fail on a semantics change upstream.

One process note

CometRegExpBenchmark is modified but the description does not include numbers. What is the speedup for an admitted pattern versus the dispatcher? That is the payoff for all of this and it should be in the description.

@sunchao

sunchao commented Aug 27, 2026

Copy link
Copy Markdown
Member

@andygrove, two factual clarifications against c62b529a for these questions:

  • The description already includes author-reported 4.6–28.8× versus the JVM dispatcher for the stated ASCII workload. Those measurements do not establish what fraction of representative real-world patterns the whitelist admits.
  • The parity suite runs Comet-enabled queries against Comet-disabled Spark results, with operator/dispatcher checks; admitted literals route to the native expression builder. It is fixed-corpus coverage, not just Java-versus-expected-values checks, but it cannot guard every possible crate semantic change.

Grammar generation and representative admission-rate measurements would add evidence beyond that coverage. The constants' structural limits are also not a compiled-byte guarantee; the existing compile-budget P2 discussion remains relevant. No tests or benchmarks were rerun for this clarification.

@andygrove

Copy link
Copy Markdown
Member

My main worry going in was that the whitelist only inspects the pattern, so I wanted to know whether the subject data could still drive Java and Rust apart — control characters and exotic whitespace especially. I spent some time testing that rather than guessing, and I think you're fine. I ported CometRegex to Java so I could fuzz it cheaply, then compiled your actual Scala file against stub SupportLevel types and checked both on 300k random strings over a metachar-heavy alphabet to confirm the port was exact (no decision mismatches, ~28k admitted). Running the analyzer-admitted patterns through java.util.regex and the regex crate against subjects built from every C0 control, DEL, NEL, NBSP, soft hyphen, all the Unicode space separators, U+2028/9, BOM, ZWSP, U+FFFD, emoji and U+10FFFF, I got zero divergences over about 68M pattern/subject pairs, and compile status agreed everywhere too, so there's no admitted pattern where Java throws and Rust doesn't or the other way round. The reason it holds looks structural to me: every whitespace and line-terminator disagreement I could find lives in a construct you reject, and the one data-sensitive construct that survives — the negated class — is code-point-based on both sides.

The one thing the pattern whitelist can't reach is invalid UTF-8 in the subject. Rust's regex treats invalid bytes as unmatchable while Java decodes them to U+FFFD first, so [^0-9] against a lone 0xC3 is true on Spark and false natively. cast_binary_to_string does a lossy decode so that path is safe, but I don't see UTF-8 validation in the native Parquet reader. This is pre-existing for every native string kernel and I'm not asking you to fix it here, but rlike is now default-on, so could you add a line to compatibility/regex.md noting that the subset assumes well-formed UTF-8?

Two smaller things. The parity suite's subject list has \n, \r, \r\n, U+0085, U+2028 and U+2029 but not tab, VT, FF, NUL, DEL, NBSP or BOM — the behavior is right, it's just that those are the characters a reader is most likely to want covered, so would you mind adding them? And nativeApplicable got widened from Literal(_, DataTypes.StringType) to Literal(v: UTF8String, _: StringType), which matches any collation. provablyCompatible guards collation but the allowIncompat && nativeApplicable(expr) branch in convert doesn't, so allowIncompatible=true on a UNICODE-collated rlike now goes native where it previously hit the dispatcher. Should the collation guard move into nativeApplicable?

The other thought is that the safety argument rests entirely on regex crate semantics, and the only parity coverage is 30 patterns going through Spark SQL. Would a table of (pattern, subject, expected) triples generated offline from Java, as a unit test in rlike.rs, be worth adding? That would catch a semantics change on a crate upgrade for almost nothing.

Last one, and for a follow-up rather than this PR: leading ^ looks safe to admit. Java's ^ outside MULTILINE and Rust's ^ without (?m) both anchor to input start only — it's $ that differs, since Java matches it before a final line terminator. I fuzzed ^-prefixed variants of all the admitted patterns against the same control-heavy subjects and saw no divergences. Given how common ^prefix is in real workloads, that seems like a decent chunk of coverage left on the table.

Nice work on this. The recursive-descent whitelist instead of substring matching is the right call, and the compile budget errs in the safe direction — collapsing {0} to 1 over-estimates rather than under-estimates what Rust actually expands.

@sunchao

sunchao commented Aug 27, 2026

Copy link
Copy Markdown
Member

Two source-level clarifications on the latest analysis, at c62b529a:

  • The lone-0xC3 example does not yet establish a native false result. RLike passes Arrow string values to regex::Regex as &str, not to the bytes-regex API. The binary-to-string cast uses JVM-compatible lossy decoding. An end-to-end reproducer needs to identify how the malformed bytes reach that kernel; this is not a claim that every malformed-input path is safe.
  • The opt-in collation widening is real: the literal matcher accepts collated string literals, and allowIncompatible=true bypasses the equivalence gate. Default routing still checks both operands' collations. Moving the guard into nativeApplicable would tighten the explicitly incompatible opt-in policy, rather than repair default routing.

These are source checks, not a rerun of the reported fuzz campaign or an executed malformed-input reproduction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Run rlike natively by default for patterns that are provably Java-regex equivalent

3 participants