-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Core: Add MetadataLog metadata table #5063
Core: Add MetadataLog metadata table #5063
Conversation
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 think this table will be quite useful especially with registerTable() to rollback to old metadata.
But I think unfortunately not all catalog will support it, like catalog without metadata json, maybe REST catalog?
@RussellSpitzer @rdblue @danielcweeks not sure if we could have a metadata table but not for all catalog types?
...2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java
Outdated
Show resolved
Hide resolved
...2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java
Outdated
Show resolved
Hide resolved
...2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java
Outdated
Show resolved
Hide resolved
4ac3c59
to
f86febd
Compare
@szehon-ho, the REST catalog can support |
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.
The table looks good to me, left a few comments. I think we could even extend this table to have more fields later, like latest schemaId, latest specId,e tc.
FYI @rdblue @RussellSpitzer if there are any concerns
...2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java
Outdated
Show resolved
Hide resolved
...2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java
Outdated
Show resolved
Hide resolved
f86febd
to
a744e3b
Compare
a744e3b
to
b58475e
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.
Thanks for the changes. Looks good to me, just a few more nits
...2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java
Show resolved
Hide resolved
...2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java
Outdated
Show resolved
Hide resolved
...2/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestMetadataTables.java
Outdated
Show resolved
Hide resolved
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.
Thanks for all the changes, just a few more really small nits and ready to go for me
Merged, thanks @singhpk234 for the contribution |
Thanks @szehon-ho, for the awesome review :) ! |
This PR aims to expose MetadataLog as a metadata table which will be beneficial, for us to figure out what the metadata.json path this table points to.
This supersedes the PR
DESC TABLE EXTENDED
#5006where we were exposing the current metadata location via
DESC TABLE EXTENDED
.cc @rdblue @jackye1995 @rajarshisarkar @amogh-jahagirdar