dwsmith1983 commented on PR #5867: URL: https://github.com/apache/datafusion-comet/pull/5867#issuecomment-5666448283
> Would it make sense for `NullGuardSupport` to cover both children of `ArrayAppend`, and to change that fixture to a nullable operand with a stateful item so it fails without the change? Yes to both. `CometArrayAppend` now declines when either child is nondeterministic, and the fixture's native case is a nullable array with a stateful item, which fails without the change on exactly the 16-row table you used. The ANSI case where the item raises on a filtered row is the same mechanism but not nondeterminism; I have noted it in the description as the remaining gap for the guard shape rather than widening the decline to every non-literal item here. > Could those queries assert `expect_dispatch(map_from_arrays)` instead, with a separate dispatcher-off file if the fallback is still worth pinning? Done: `map_from_arrays_dedup_policy.sql` asserts the dispatched route with the dispatcher on, and `map_from_arrays_dedup_policy_dispatcher_off.sql` pins the fallback. > an `array<int>` column in at least the `size` and `map_from_arrays` fixtures would be worth having Added to both: the dispatched kernel now reads its arrays from columns as well as building them inline. `cardinality` says Hybrid. > Do you have numbers for the routing here? Not yet; I will run the same shape #5875 measured before this lands. On #5854, once native LAST_WIN lands the `map_from_arrays` dispatch branch goes away and only the dispatcher-off fixture needs to follow, so I am fine with #5854 landing first. -- 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]
