sam-1112 commented on PR #5215: URL: https://github.com/apache/datafusion-comet/pull/5215#issuecomment-5595411379
Thanks for the thorough re-review and for checking the additional edge cases against both this branch and main. The compatibility checks against current main and the benchmark measurements were especially helpful. I’ve addressed the three inline comments: - Updated both the direct and dictionary Utf8View cases to use "Rhododendrons" (13 bytes), so they exercise the out-of-line view representation. - Added coverage for Dictionary(UInt64, Utf8) and a sliced dictionary with a non-zero offset. - Reworded the null-buffer assertion comment to describe preserving the null-buffer-free output shape and avoiding the validity allocation, without referring to the filter fast path. The benchmark results and methodology are now included in the PR description. The targeted local checks still pass: - cargo fmt --check - cargo test -p datafusion-comet-spark-expr --lib rlike - cargo clippy -p datafusion-comet-spark-expr --lib --tests -- -D warnings Thanks again for the detailed validation and for confirming that the iterator-based implementation is a reasonable performance trade-off. -- 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]
