Repository navigation
GH-55404: [C++] Fix coalesce exact dispatch for mixed dictionary types - #55405
Open
zanmato1984 wants to merge 1 commit into
Open
zanmato1984 wants to merge 1 commit into
zanmato1984 wants to merge 1 commit into
Conversation
…y types Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
|
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the dictionary-type dispatch change.
No open issues 🎉
What changed in this PR
Fixes GH-55404 by requiring identical dictionary types for exact coalesce dispatch. Mixed dictionary inputs can then use the existing decoding and type-promotion path during expression binding, while identical types retain dictionary-encoded execution.
Changes:
- Require full dictionary type equality for the dictionary kernel.
- Add exact- and best-dispatch tests for differing index types, value types, and ordering flags.
- Add expression binding and execution tests covering reversed arguments, variadic calls, and identical types with different dictionary contents.
| File | Description |
|---|---|
cpp/src/arrow/compute/kernels/scalar_if_else.cc |
Enforces identical dictionary types during exact dispatch. |
cpp/src/arrow/compute/kernels/scalar_if_else_test.cc |
Tests exact dispatch rejection and best-dispatch fallback. |
cpp/src/arrow/compute/expression_test.cc |
Tests cast insertion, execution results, and dictionary-preserving behavior. |
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
The
coalescedictionary kernel accepts arguments with different index types, value types, or ordered flags during exact dispatch. Expression binding therefore skips dictionary decoding and type promotion, and the bound expression fails with aTypeErrorat execution.What changes are included in this PR?
Require full dictionary type equality using
AllTypesAreIdentical(). Mixed dictionary arguments follow the existing best-dispatch and cast-insertion path, while identical dictionary types retain dictionary-encoded execution.Are these changes tested?
Added exact/best-dispatch and expression binding/execution tests covering differences in index types, value types, and ordering, both argument orders, and variadic calls. Same-type inputs with different dictionary contents verify that dictionary-encoded execution is preserved.
Are there any user-facing changes?
Bound
coalesceexpressions with compatible mixed dictionary inputs now execute successfully, returning decoded values promoted to a common type where needed.Was AI used for this PR?
AI generated the implementation, tests, and PR description. The author conducted a thorough review with AI assistance.
PR code and description written by:
Reviewed before submission by: