sam-1112 opened a new pull request, #5415: URL: https://github.com/apache/datafusion-comet/pull/5415
## 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](https://github.com/apache/datafusion-comet/issues/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, escaped metacharacters. - **Rejected:** `^` `$` `.` `\d` `\w` `\s`, lookaround, backrefs, possessive/lazy, inline flags, `\p` / `\u` / `\0`, nested classes, non-ASCII in the pattern, and non-default Spark 4 collation on either the subject or the pattern (the operands that feed regex evaluation). - `CometRLike` consults the analyzer. In-subset literals become `Compatible()` (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 for any non-null literal. - Docs: `compatibility/regex.md`, `expressions.md` (`rlike` / `regexp` / `regexp_like`), and the `rlike` audit note. - This first PR covers **rlike only**. No pattern rewriting (e.g. `(?-u)`), no collation propagation into the native kernel, and no change to `regexp_replace` / `split` / `regexp_extract` / `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, including lexer-boundary cases (`[(?=]`, `\\d`, `[.]`) and scanner edges (`\A`/`\Z`, `{2,1}`, unclosed `(`). - `CometRegexParitySuite`: every admitted pattern × ASCII / non-ASCII / newline / NULL subjects; native results compared with `java.util.regex.Pattern.find`. The test also asserts that EXPLAIN output does not contain `JVM codegen dispatcher: rlike`. Multi-batch: 5000 rows, batch size 64. NULL *subjects* match Spark; a NULL *pattern* is not in this native corpus. - `CometRegExpJvmSuite`: routing (native default, dispatcher default, opt-in, non-literal, NULL pattern stays on the dispatcher with unchanged all-null results, dispatcher-off in-subset still native, dispatcher-off out-of-subset falls back to Spark, invalid regex, Java-only patterns with `allowIncompatible=true` including the expected native compile failure for constructs unsupported by Rust, Spark 4 collation on subject and on pattern). - SQL file `rlike_auto_native.sql` for default-config result equality. - Validated locally with Spark 3.5.9 and 4.1.3 on JDK 17. `CometRegExpBenchmark`, 1,048,576 rows, Apple M4, JDK 17. Native vs Spark (in-subset patterns; after this PR the Comet default is native): | 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, using 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 by default. | 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 dispatcher to native is the speedup that matters for this PR. The native implementation matches over Arrow buffers and is expected to avoid the dispatcher's per-row `toString()` / `Matcher` allocations; that allocation difference was not measured with JFR or a GC log in this PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
