Skip to content

feat: expand arrow type conversion test #1295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

jdockerty
Copy link
Contributor

@jdockerty jdockerty commented May 6, 2025

Which issue does this PR close?

Helps #1277

What changes are included in this PR?

This expands the test_type_conversion test with the specific DataType::Dictionary type.

Props to @phillipleblanc for this, as I noticed the extra coverage on his PR which I believe is very useful to capture1.

Footnotes

  1. I was looking to add similar to https://github.com/apache/iceberg-rust/pull/1293, but it was merged just before I pushed the changes 😆

Copy link
Contributor

@phillipleblanc phillipleblanc left a comment

Choose a reason for hiding this comment

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

Thanks @jdockerty!

@jdockerty
Copy link
Contributor Author

@liurenjie1024

This was a follow-up from the previous change, would you mind taking a quick look at this one?

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thank you @jdockerty for working on this. Nice change to me.

@Xuanwo Xuanwo merged commit 8c2f9f2 into apache:main May 10, 2025
17 checks passed
@jdockerty jdockerty deleted the feat/iceberg-arrow-conversion-test-expansion branch May 10, 2025 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants