fix checking physical type for Decimal type in StatsAggregator#2515
Merged
kevinjqliu merged 5 commits intoapache:mainfrom Oct 26, 2025
Merged
fix checking physical type for Decimal type in StatsAggregator#2515kevinjqliu merged 5 commits intoapache:mainfrom
kevinjqliu merged 5 commits intoapache:mainfrom
Conversation
4 tasks
desmondcheongzx
pushed a commit
to Eventual-Inc/Daft
that referenced
this pull request
Sep 25, 2025
## Changes Made Added changes to conditionally handle breaking changes in pyiceberg 0.10.0 for how to initialize `DataFile` and `Record` and constraints on `name` for `Schema` fields and `PartitionField`. I did not update dev dependencies at this point, because there are actually still issues with Decimal. I have a [PR](apache/iceberg-python#2515) open that hopefully addresses this. To test, I had to temporarily change dev dependencies to 0.10.0, and run tests without decimal type. I need to update to 0.10.0 because support for anonymous was added (needed on my side). Note sure what the best path is here considering that pyiceberg releases on a much slower cadence. ## Related Issues Closes #5223 ## Checklist - [ ] Documented in API Docs (if applicable) - [ ] Documented in User Guide (if applicable) - [ ] If adding a new documentation page, doc is added to `docs/mkdocs.yml` navigation - [ ] Documentation builds and is formatted properly (tag @/ccmao1130 for docs review)
Contributor
|
Hey @gmweaver thanks for adding this. Could you also add a test for this? This ensures that we don't break it in the future. |
Contributor
Author
|
@Fokko test added and ensured I ran |
kevinjqliu
reviewed
Sep 26, 2025
Contributor
kevinjqliu
left a comment
kevinjqliu
left a comment
There was a problem hiding this comment.
iceberg-python/pyiceberg/io/pyarrow.py
Lines 2507 to 2518 in e5e7453
can we simplify this logic too? can be a follow up PR
kevinjqliu
reviewed
Sep 26, 2025
kevinjqliu
reviewed
Sep 26, 2025
kevinjqliu
approved these changes
Sep 26, 2025
Contributor
kevinjqliu
left a comment
kevinjqliu
left a comment
There was a problem hiding this comment.
LGTM Thanks for fixing this issue.
added 3 commits
September 26, 2025 14:43
This was referenced Feb 12, 2026
desmondcheongzx
added a commit
to Eventual-Inc/Daft
that referenced
this pull request
Feb 18, 2026
## Changes Made Makes required changes to support pyiceberg v0.11.0 which now properly supports Decimal type (see apache/iceberg-python#2515). Excludes v0.9.1 and v0.10.0 that have the Decimal type issue. There is a breaking change in pyiceberg that requires a partition field to have a different name than any field in the schema. Updates to automatically name field by using internal `pyiceberg` naming conventions for partition fields. Specific changes required: - Use `_PartitionNameGenerator` in `pyiceberg` to generate the partition field name based on the source field and field ID when creating a table. - Fix `can_cast_types` to handle `Extension`. - Update `Dockerfile` for iceberg integration tests to use `0.11.0`. - Update `get_data_files` to use pyiceberg `InspectTable` to get data files as opposed to using the now position-based `Record`. ## Related Issues <!-- Link to related GitHub issues, e.g., "Closes #123" --> --------- Co-authored-by: gmweaver <gmweaver.usc@gmail.com> Co-authored-by: desmondcheongzx <desmondcheongzx@gmail.com>
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
3 participants
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
Fix for #2057. It looks like the initial fix #1839 might have missed updating here to handle. I could use feedback on if this is the best fix, it is at least simple.
Are these changes tested?
Are there any user-facing changes?
No