Skip to content

API: Add UnknownType#12012

Merged
Fokko merged 7 commits into
apache:mainfrom
Fokko:fd-add-unknown
Jan 24, 2025
Merged

API: Add UnknownType#12012
Fokko merged 7 commits into
apache:mainfrom
Fokko:fd-add-unknown

Conversation

@Fokko

@Fokko Fokko commented Jan 20, 2025

Copy link
Copy Markdown
Contributor

Fixes #11732

@Fokko Fokko requested a review from HonahX January 20, 2025 18:37
Comment thread api/src/test/java/org/apache/iceberg/types/TestSerializableTypes.java Outdated
Comment thread api/src/main/java/org/apache/iceberg/transforms/Identity.java Outdated
Comment thread api/src/test/java/org/apache/iceberg/transforms/TestBucketing.java
@HonahX HonahX linked an issue Jan 22, 2025 that may be closed by this pull request
3 tasks
Comment thread api/src/main/java/org/apache/iceberg/transforms/Identity.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/TestSortOrder.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/TestSortOrder.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/TestSortOrder.java Outdated

@RussellSpitzer RussellSpitzer left a comment

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.

I'm ok allowing Unknown in all these transforms and such, but I don't think we actually have a case where it can happen.

@HonahX HonahX left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@nastra nastra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with one minor comment around testing

@Fokko Fokko merged commit a04220a into apache:main Jan 24, 2025
@Fokko

Fokko commented Jan 24, 2025

Copy link
Copy Markdown
Contributor Author

Thanks @HonahX @RussellSpitzer and @nastra for the review 🚀

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support UnknownType in API Support UnknownType for V3 Schema

4 participants