Skip to content
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,Format: Deprecate embedded manifests #11586

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Fokko
Copy link
Contributor

@Fokko Fokko commented Nov 19, 2024

The embedded manifests are not used outside of the company where Iceberg found its inception. I think it is a good thing to officially deprecate this field, and be able to remove the DummyFileIO from the Java codebase at some point.

@github-actions github-actions bot added core Specification Issues that may introduce spec changes. labels Nov 19, 2024
@Fokko Fokko force-pushed the fd-deprecate-manifests branch 2 times, most recently from 77138b8 to bfa9560 Compare November 19, 2024 11:54
format/spec.md Show resolved Hide resolved
Comment on lines 167 to 168
LOG.warn(
"Support for embedded manifests are deprecated since Iceberg 1.8.0, will be removed in either 1.9.0 or 2.0.0");
Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar Nov 19, 2024

Choose a reason for hiding this comment

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

Hm, I'm a bit concerned about deprecating the ability to read embedded manifests.

I certainly think it makes sense to stop writing them, but there may still be some v1 tables out there that we don't want to break reads on those in a later Iceberg version.

@@ -81,6 +84,9 @@ static void toJson(Snapshot snapshot, JsonGenerator generator) throws IOExceptio
// write just the location. manifests should not be embedded in JSON along with a list
generator.writeStringField(MANIFEST_LIST, manifestList);
} else {
LOG.warn(
"Support for embedded manifests are deprecated since Iceberg 1.8.0, will be removed in either 1.9.0 or 2.0.0");
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: do we plan to go from 1.9.0 to 2.0.0, instead of 1.10.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's up to the community :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Specification Issues that may introduce spec changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants