-
Notifications
You must be signed in to change notification settings - Fork 0
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
USD-2054 Upgrade Iceberg to 1.6.1 #8
base: main
Are you sure you want to change the base?
Conversation
…ormat and therefore ts_us must lose its precision
73b4897
to
33b3479
Compare
@@ -179,7 +179,7 @@ public void testCommitResponseBecomesDataWrittenPartitioned() { | |||
Types.NestedField.optional( | |||
10_304, | |||
"delete_files", | |||
Types.ListType.ofRequired(10_304, DataFile.getType(spec.partitionType()))))); | |||
Types.ListType.ofRequired(10_305, DataFile.getType(spec.partitionType()))))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need to change tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find the exact change in Iceberg but it looks like somewhere there was a change adding stricter validation for schema field IDs to prevent ID collisions and therefore the parent and optional field will now have different IDs.
Closed PR on main to upgrade Iceberg: databricks#313
@@ -265,7 +265,7 @@ public void testShouldDeduplicateDeleteFilesBeforeAppending() { | |||
Assertions.assertEquals(1, snapshots.size()); | |||
|
|||
Snapshot snapshot = snapshots.get(0); | |||
Assertions.assertEquals(DataOperations.OVERWRITE, snapshot.operation()); | |||
Assertions.assertEquals(DataOperations.DELETE, snapshot.operation()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here - did some functionality change that required us to change tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Iceberg 1.6.0 there was a change that means this is now a delete, I believe this is the PR: apache/iceberg#10123
Closed PR on main to upgrade Iceberg: databricks#313
https://rapid7.atlassian.net/browse/USD-2054
Includes: