zanmato1984 commented on PR #50501: URL: https://github.com/apache/arrow/pull/50501#issuecomment-5247537812
@pitrou @LarryHu0217 I think the generic direction is the right one, but I don't think the eager `ScalarFunction::Execute` fallback is the right layer for it. My preference would be to keep `DispatchBest` / `DispatchExact` expressing only real kernels, and implement this as an internal executor-level adapter instead: first try the normal executor so existing dictionary kernels still take precedence; if that fails for a fixed-arity pure unary dictionary input whose value type has a matching kernel, construct a specialized dictionary-unary executor that runs the value kernel and remaps the result through the indices. That keeps the dictionary-specific execution logic out of the top-level eager `Execute` path while still avoiding full logical decoding. The other point I would want to nail down before merging is semantic equivalence with the decoded-input path. If the fallback executes over the entire dictionary, it can evaluate unused dictionary values and change error behavior. For example, `strptime` with `error_is_null=false` should not fail because of an invalid dictionary value that is never referenced by the indices. So I think the adapter needs to process only referenced dictionary values, or otherwise preserve decoded-input behavior, and add a regression test for that case. So my answer is: generic support makes sense, but I would move it out of `ScalarFunction::Execute` into a dedicated executor-level adapter and tighten the semantics/tests before merging. -- 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]
