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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/src/main/java/org/apache/iceberg/BaseSnapshot.java
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ class BaseSnapshot implements Snapshot {
this.v1ManifestLocations = null;
}

/**
* Constructor with embedded manifests
*
* @deprecated since 1.8.0, will be removed 2.0.0
*/
@Deprecated
BaseSnapshot(
long sequenceNumber,
long snapshotId,
Expand Down
9 changes: 9 additions & 0 deletions core/src/main/java/org/apache/iceberg/SnapshotParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
import org.apache.iceberg.util.JsonUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class SnapshotParser {
private static final Logger LOG = LoggerFactory.getLogger(SnapshotParser.class);

private SnapshotParser() {}

Expand Down Expand Up @@ -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 2.0.0");

// embed the manifest list in the JSON, v1 only
JsonUtil.writeStringArray(
MANIFESTS,
Expand Down Expand Up @@ -158,6 +164,9 @@ static Snapshot fromJson(JsonNode node) {
manifestList);

} else {
LOG.warn(
"Support for embedded manifests are deprecated since Iceberg 1.8.0, will be removed in 2.0.0");

// fall back to an embedded manifest list. pass in the manifest's InputFile so length can be
// loaded lazily, if it is needed
return new BaseSnapshot(
Expand Down
22 changes: 11 additions & 11 deletions format/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -654,17 +654,17 @@ The `first_row_id` is only inherited for added data files. The inherited value m

A snapshot consists of the following fields:

| v1 | v2 | v3 | Field | Description |
| ---------- | ---------- |------------|------------------------------|------------------------------------------------------------------------------------------------------------------------------------|
| _required_ | _required_ | _required_ | **`snapshot-id`** | A unique long ID |
| _optional_ | _optional_ | _optional_ | **`parent-snapshot-id`** | The snapshot ID of the snapshot's parent. Omitted for any snapshot with no parent |
| | _required_ | _required_ | **`sequence-number`** | A monotonically increasing long that tracks the order of changes to a table |
| _required_ | _required_ | _required_ | **`timestamp-ms`** | A timestamp when the snapshot was created, used for garbage collection and table inspection |
| _optional_ | _required_ | _required_ | **`manifest-list`** | The location of a manifest list for this snapshot that tracks manifest files with additional metadata |
| _optional_ | | | **`manifests`** | A list of manifest file locations. Must be omitted if `manifest-list` is present |
| _optional_ | _required_ | _required_ | **`summary`** | A string map that summarizes the snapshot changes, including `operation` as a _required_ field (see below) |
| _optional_ | _optional_ | _optional_ | **`schema-id`** | ID of the table's current schema when the snapshot was created |
| | | _optional_ | **`first-row-id`** | The first `_row_id` assigned to the first row in the first data file in the first manifest, see [Row Lineage](#row-lineage) |
| v1 | v2 | v3 | Field | Description |
| ---------- | ---------- |------------|------------------------------|-------------------------------------------------------------------------------------------------------------------------------------|
| _required_ | _required_ | _required_ | **`snapshot-id`** | A unique long ID |
| _optional_ | _optional_ | _optional_ | **`parent-snapshot-id`** | The snapshot ID of the snapshot's parent. Omitted for any snapshot with no parent |
| | _required_ | _required_ | **`sequence-number`** | A monotonically increasing long that tracks the order of changes to a table |
| _required_ | _required_ | _required_ | **`timestamp-ms`** | A timestamp when the snapshot was created, used for garbage collection and table inspection |
| _optional_ | _required_ | _required_ | **`manifest-list`** | The location of a manifest list for this snapshot that tracks manifest files with additional metadata |
| _optional_ | | | ~~**`manifests`**~~ | ~~A list of manifest file locations. Must be omitted if `manifest-list` is present.~~ (**Deprecated**: use `manifest-list` instead) |
Fokko marked this conversation as resolved.
Show resolved Hide resolved
| _optional_ | _required_ | _required_ | **`summary`** | A string map that summarizes the snapshot changes, including `operation` as a _required_ field (see below) |
| _optional_ | _optional_ | _optional_ | **`schema-id`** | ID of the table's current schema when the snapshot was created |
| | | _optional_ | **`first-row-id`** | The first `_row_id` assigned to the first row in the first data file in the first manifest, see [Row Lineage](#row-lineage) |

The snapshot summary's `operation` field is used by some operations, like snapshot expiration, to skip processing certain snapshots. Possible `operation` values are:

Expand Down
Loading