Skip to content

GH-40128: [C++][Python] Preserve dictionaries in empty casts - #50837

Open
LarryHu0217 wants to merge 2 commits into
apache:mainfrom
LarryHu0217:codex/preserve-empty-dictionary-cast-40128
Open

GH-40128: [C++][Python] Preserve dictionaries in empty casts#50837
LarryHu0217 wants to merge 2 commits into
apache:mainfrom
LarryHu0217:codex/preserve-empty-dictionary-cast-40128

Conversation

@LarryHu0217

@LarryHu0217 LarryHu0217 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Run dictionary-to-dictionary casts through the cast kernel for zero-length inputs when the dictionary type changes.
  • Add a regression test that verifies both the widened index type and preserved dictionary values.

The generic zero-length scalar fast path creates an empty output dictionary and discards categories that are still attached to the input array. Other zero-length scalar operations continue to use the existing fast path.

Testing

  • cpp/build-40128-auto/debug/arrow-compute-scalar-cast-test (109 tests passed)
  • clang-format --dry-run --Werror on both changed files
  • git diff --check

AI assistance

I used Codex to help inspect the executor path, prepare the focused change, and draft the regression test. I reviewed the final diff, reproduced the failure before the fix, and ran the tests above locally.

Closes #40128

@uros-b

uros-b commented Aug 10, 2026

Copy link
Copy Markdown
Member

Fix looks narrowly scoped and properly tested, thank you @LarryHu0217!

@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Aug 10, 2026
@pitrou

pitrou commented Aug 31, 2026

Copy link
Copy Markdown
Member

@uros-b We like to have PR reviews as much as possible, but since you are not a contributor to this codebase I wonder if it wouldn't be more productive to start contributing (assuming you're interested in doing so, of course).

In general, I would recommend exercising caution if you're not familiar with the codebase and feel the desire to post a PR review.

Comment on lines +788 to +792
const bool changes_dictionary_type =
batch.num_values() == 1 && batch.values[0].type() != nullptr &&
is_dictionary(batch.values[0].type()->id()) &&
is_dictionary(output_type_.type->id()) &&
!batch.values[0].type()->Equals(*output_type_.type);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks... weird. Why the type unequality condition?

Also, this will trigger on every scalar compute function, not just the cast function.

@pitrou

pitrou commented Aug 31, 2026

Copy link
Copy Markdown
Member

@zanmato1984 Could you perhaps take a look? I'm not sure how to make this focused enough to apply to the Cast function, not every other scalar function that takes a dict as input.

(also it's not obvious we want to fix this, though it would be useful for some use cases)

@uros-b

uros-b commented Aug 31, 2026

Copy link
Copy Markdown
Member

Thank you @pitrou! Makes sense, sorry for the noise here. I'll also continue monitoring this PR too, and see how it continues evolving with additional feedback / comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Python] Casting an empty dictionary array loses category information

3 participants