-
Notifications
You must be signed in to change notification settings - Fork 53
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
feat(catalog): Standardize Catalog create table function #245
Conversation
984351a
to
b384c0f
Compare
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.
generally LGTM, have a few nit comments
Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>
@Fokko any further comments? |
return &metadataV1{ | ||
commonMetadata: common, | ||
Schema: freshSchema, | ||
Partition: slices.Collect(freshPartitions.Fields()), |
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 PyIceberg we decided to upgrade everything to V2 internally, and do the V1-specific things when serializing/deserializing. This is not a blocker for this PR.
Thanks @zeroshade 🙌 |
Adding a CreateTable function to the
Catalog
interface, standardizing the implementation that was initially created by #146 so that it isn't specific to the REST catalog and can be implemented by any catalog.This also requires adding the
AssignFresh*IDs
functions so that the REST catalog implementation is correctly re-assigning IDs as other implementations do.