Skip to content

Conversation

@jun-he
Copy link
Collaborator

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

The field ID in the new spec should start with the largest last assigned partition id among all table specs.

@github-actions github-actions bot added the core label Jan 13, 2021
@jun-he jun-he force-pushed the jun/fix-spec-field-id branch from 21cb153 to d032221 Compare January 13, 2021 08:29
Copy link
Contributor

@yyanyy yyanyy left a comment

Choose a reason for hiding this comment

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

Thank you for catching this!

@rdblue
Copy link
Contributor

rdblue commented Jan 13, 2021

Good catch. We also still need to add a lastAssignedFieldId to table metadata for this, like we have for last assigned column ID.

@rdblue rdblue merged commit bfe1bfc into apache:master Jan 13, 2021
@yyanyy
Copy link
Contributor

yyanyy commented Jan 13, 2021

Good catch. We also still need to add a lastAssignedFieldId to table metadata for this, like we have for last assigned column ID.

Is this just to ensure implementation correctness per spec? As it seems that from the current implementation getting this from PartitionSpec::lastAssignedFieldId should be good enough, as there's no table operation to actually remove a partition field from specs?

@jun-he jun-he deleted the jun/fix-spec-field-id branch January 14, 2021 07:47
XuQianJin-Stars pushed a commit to XuQianJin-Stars/iceberg that referenced this pull request Mar 22, 2021
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