-
Notifications
You must be signed in to change notification settings - Fork 229
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
Bug Fix: Use historical partition field name #1161
Conversation
pyiceberg/table/update/spec.py
Outdated
@@ -280,7 +280,7 @@ def _partition_field(self, transform_key: Tuple[int, Transform[Any, Any]], name: | |||
for field_key in historical_fields: | |||
if field_key[0] == source_id and field_key[2] == repr(transform): | |||
if name is None or field_key[3] == name: | |||
return PartitionField(source_id, field_key[1], transform, name) | |||
return PartitionField(source_id, field_key[1], transform, field_key[3]) |
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.
What do you think about using an Enum to get the values from the field_key? I think that will make it easier to understand what each position means.
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.
Hey, thank you for the great suggestion! I didn't use Enums, but updated it to use a list of PartitionField instead, which I think makes a lot more sense than the current approach.
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.
lgtm, thanks for adding the tests to show the difference between v1 and v2 spec
* use historical partition field name * thanks ndrluis!
* use historical partition field name * thanks ndrluis!
Fixes: #1131