andygrove commented on PR #5817: URL: https://github.com/apache/datafusion-comet/pull/5817#issuecomment-5609013194
Thanks @sunchao. You're right about the pins, and I've corrected the description. `branch-1.0` resolves DataFusion 54.1.0 and Arrow 58.4.0 while `main` at `92ad99e9` is on 55.0.0 and Arrow 59.3.0, so the "same DataFusion version" line was just wrong. The cherry-pick still applied unmodified because the Arrow surface these three files touch didn't change between those versions, which is what the description now says instead. Your review also caught a second inaccuracy I'd written: I claimed the uniformity check replaced the column-index test, but it's ANDed onto it in `utils.rs`, so the fast path is strictly narrower than before rather than differently scoped. That's reworded too. On the microbenchmark, I haven't run one against the 58.4 dependency set, and the description now says so rather than implying the numbers carry over from #5757. Would you rather I produce that before this merges, or is it reasonable to land the correctness fix here given the patch is byte-identical to what's already on `main` and take the benchmark separately? -- 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]
