yinli-systems commented on PR #11062:
URL: https://github.com/apache/arrow-rs/pull/11062#issuecomment-5676070962
Thanks @Jefffrey; I agree this needs an explicit encoded-type decision
before merge. I checked the current layers: direct
`arrow_arith::numeric::{add,sub,mul,div,neg,...}` dispatch accepts primitives
but not `Dictionary`; DataFusion already has dictionary arithmetic tests such
as `plus_op_dict`, with its coercion/evaluation layer producing dense primitive
results. The DataFusion #24565 change is about REE coercion and window range
extraction, not evidence that direct Arrow numeric kernels support
dictionaries. Conversely, `arrow_arith::temporal::date_part` handles both
Dictionary and REE by transforming physical values, so there does not appear to
be a universal policy forbidding encoded inputs in Arrow kernels.
The narrow capability in this PR is compression-preserving `REE x scalar`
and `REE x REE` arithmetic, where the output remains REE and work scales with
runs. `REE x dense non-scalar` is rejected rather than silently expanding to
logical length. Dictionary arithmetic has different cardinality/key-pair
questions, so I would treat its direct-kernel contract as a separate design
item instead of adding an unreviewed implementation here. I corrected the PR
description, which had implied a stronger dictionary precedent than actually
exists.
My suggested decision is: accept this narrow REE contract as an opt-in
encoded kernel capability, then specify Dictionary and mixed dense/encoded
semantics in follow-up work. If maintainers prefer consistent direct-kernel
support for both encodings before this lands, I am happy to pause or rescope
#11062 after that direction is clear.
--
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]