Skip to content

Conversation

@jun-he
Copy link
Collaborator

@jun-he jun-he commented Jan 31, 2021

Follow up of #2089

@github-actions github-actions bot added the core label Jan 31, 2021
}

Integer lastAssignedPartitionId = JsonUtil.getIntOrNull(LAST_ASSIGNED_PARTITION_ID, node);
Integer lastAssignedPartitionId = JsonUtil.getIntOrNull(LAST_PARTITION_ID, node);
Copy link
Member

Choose a reason for hiding this comment

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

Won't this break backwards compatibility?

Copy link
Contributor

Choose a reason for hiding this comment

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

This property was just added, so the rename is safe.

this.nameToField = indexSpecByName(spec);
this.transformToField = indexSpecByTransform(spec);
this.lastAssignedPartitionId = base.lastAssignedPartitionId();
this.lastAssignedPartitionId = base.lastPartitionId();
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to rename the method, just the property in the metadata file.

@rdblue
Copy link
Contributor

rdblue commented Feb 1, 2021

@jun-he, can you roll back the field and and method name changes in other classes? I think we only need to change the table metadata property, not the others.

@jun-he
Copy link
Collaborator Author

jun-he commented Feb 1, 2021

@rdblue Thanks for the review and updated the PR accordingly.

@rdblue rdblue merged commit d80ff56 into apache:master Feb 1, 2021
@rdblue
Copy link
Contributor

rdblue commented Feb 1, 2021

Thanks, @jun-he! Looks good.

@jun-he jun-he deleted the jun/rename-partition-id branch February 2, 2021 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants