sam-1112 commented on PR #5415: URL: https://github.com/apache/datafusion-comet/pull/5415#issuecomment-5455467091
Thanks @andygrove and @sunchao for the detailed review, extensive fuzzing, and clarifications. I addressed the concrete items in `136ca137`: - Added the missing control-heavy parity subjects: TAB, VT, FF, NUL, DEL, NBSP, and BOM. - Took the conservative collation approach by moving the non-default-collation guard into `nativeApplicable`. Collated `rlike` expressions therefore remain on the JVM dispatcher even when `allowIncompatible=true`. I also added regression tests covering both collated subjects and collated patterns. - Documented that the compile-budget constants are conservative structural heuristics, not compiled-byte estimates or guarantees relative to the Rust `regex` crate's 10 MiB limit. - Added cautious compatibility documentation stating that native `rlike` assumes well-formed UTF-8. I avoided claiming a proven end-to-end malformed-byte divergence because that path has not yet been reproduced through an actual native input path. - Updated the PR description to reflect the current implementation and tests, including the collation behavior, additional parity subjects, compile-budget heuristics, UTF-8 note, and dispatcher-versus-native benchmark results. The parity suite exercises the native Rust kernel rather than comparing Java results only against static expected values. I agree that the collation change tightens the explicit opt-in path rather than fixing the default routing, since the default path was already protected by `provablyCompatible`. To keep this PR focused, I suggest leaving the following as follow-up work: - grammar-based generative or nightly fuzzing, - measuring admission coverage against a representative real-world regex corpus, - adding an offline Java-generated `(pattern, subject, expected)` table to the Rust unit tests, - reproducing malformed UTF-8 through an actual native input path, and - evaluating support for leading `^`. Thanks again for the thorough review. -- 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]
