sam-1112 commented on PR #5215: URL: https://github.com/apache/datafusion-comet/pull/5215#issuecomment-5169582971
Thanks for the review and for kicking CI past the first-time-contributor gate. On the dictionary arm: agreed — I've switched to `as_any_dictionary()` so non-`Int32` keys are handled, and added unit coverage for `Dictionary(Int32, Utf8)`, `Dictionary(Int32, Utf8View)`, and `Dictionary(Int8, Utf8)`. On reachability: I don't currently have a known plan that feeds `Utf8View` (or `LargeUtf8`) into native `RLike`. As you noted, `planner.rs` still casts `Utf8View` results back to `Utf8` because Comet does not yet support view types, and the existing `rlike_rust.sql` path uses plain Spark `string` / Parquet, which shows up as `Utf8`. So this change is mainly hardening for the layouts #5102 called out, plus dictionary key-type correctness, rather than fixing a query I can reproduce end-to-end today. If we later find a reachable config/path, I'm happy to add an e2e case to `rlike_rust.sql` in a follow-up. I've also updated the PR for the other inline comments (`internal_err!`, docs cleanup, test consolidation, and the `iter`/`collect` simplification). -- 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]
