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

OpenAPI: Deprecate snapshot-id of SetStatisticsUpdate #12010

Merged
merged 7 commits into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from 6 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
13 changes: 13 additions & 0 deletions api/src/main/java/org/apache/iceberg/UpdateStatistics.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,22 @@ public interface UpdateStatistics extends PendingUpdate<List<StatisticsFile>> {
* the snapshot if any exists.
*
* @return this for method chaining
* @deprecated since 1.8.0, will be removed 1.9.0 or 2.0.0, use {@link
* #setStatistics(StatisticsFile)}.
*/
@Deprecated
UpdateStatistics setStatistics(long snapshotId, StatisticsFile statisticsFile);

/**
* Set the table's statistics file for given snapshot, replacing the previous statistics file for
* the snapshot if any exists.
*
* @return this for method chaining
*/
Fokko marked this conversation as resolved.
Show resolved Hide resolved
default UpdateStatistics setStatistics(StatisticsFile statisticsFile) {
throw new UnsupportedOperationException("Setting statistics is not supported");
}

/**
* Remove the table's statistics file for given snapshot.
*
Expand Down
17 changes: 13 additions & 4 deletions core/src/main/java/org/apache/iceberg/MetadataUpdate.java
Original file line number Diff line number Diff line change
Expand Up @@ -231,16 +231,25 @@ public void applyTo(TableMetadata.Builder metadataBuilder) {
}

class SetStatistics implements MetadataUpdate {
private final long snapshotId;
private final StatisticsFile statisticsFile;

/**
* Set statistics for a snapshot.
*
* @deprecated since 1.8.0, will be removed in 1.9.0 or 2.0.0, use
* SetStatistics(statisticsFile).
*/
@Deprecated
public SetStatistics(long snapshotId, StatisticsFile statisticsFile) {
this.snapshotId = snapshotId;
this.statisticsFile = statisticsFile;
}

public SetStatistics(StatisticsFile statisticsFile) {
this.statisticsFile = statisticsFile;
}

public long snapshotId() {
return snapshotId;
return statisticsFile.snapshotId();
}

public StatisticsFile statisticsFile() {
Expand All @@ -249,7 +258,7 @@ public StatisticsFile statisticsFile() {

@Override
public void applyTo(TableMetadata.Builder metadataBuilder) {
metadataBuilder.setStatistics(snapshotId, statisticsFile);
metadataBuilder.setStatistics(statisticsFile);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,10 +512,9 @@ private static MetadataUpdate readSetDefaultSortOrder(JsonNode node) {
}

private static MetadataUpdate readSetStatistics(JsonNode node) {
long snapshotId = JsonUtil.getLong(SNAPSHOT_ID, node);
JsonNode statisticsFileNode = JsonUtil.get(STATISTICS, node);
StatisticsFile statisticsFile = StatisticsFileParser.fromJson(statisticsFileNode);
return new MetadataUpdate.SetStatistics(snapshotId, statisticsFile);
return new MetadataUpdate.SetStatistics(statisticsFile);
}

private static MetadataUpdate readRemoveStatistics(JsonNode node) {
Expand Down
15 changes: 14 additions & 1 deletion core/src/main/java/org/apache/iceberg/SetStatistics.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,23 @@ public SetStatistics(TableOperations ops) {
this.ops = ops;
}

/**
* Set the statistics file for a snapshot.
*
* @deprecated since 1.8.0, will be removed in 1.9.0 or 2.0.0, use {@link
* #setStatistics(StatisticsFile)}.
*/
@Deprecated
@Override
public UpdateStatistics setStatistics(long snapshotId, StatisticsFile statisticsFile) {
Preconditions.checkArgument(snapshotId == statisticsFile.snapshotId());
statisticsToSet.put(snapshotId, Optional.of(statisticsFile));
statisticsToSet.put(statisticsFile.snapshotId(), Optional.of(statisticsFile));
return this;
}

@Override
public UpdateStatistics setStatistics(StatisticsFile statisticsFile) {
statisticsToSet.put(statisticsFile.snapshotId(), Optional.of(statisticsFile));
return this;
}

Expand Down
15 changes: 14 additions & 1 deletion core/src/main/java/org/apache/iceberg/TableMetadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -1319,6 +1319,12 @@ private Builder resetMainBranch() {
return this;
}

/**
* Set a statistics file for a snapshot.
*
* @deprecated since 1.8.0, will be removed 1.9.0 or 2.0.0, use setStatistics(statisticsFile).
*/
@Deprecated
public Builder setStatistics(long snapshotId, StatisticsFile statisticsFile) {
Preconditions.checkNotNull(statisticsFile, "statisticsFile is null");
Preconditions.checkArgument(
Expand All @@ -1327,7 +1333,14 @@ public Builder setStatistics(long snapshotId, StatisticsFile statisticsFile) {
snapshotId,
statisticsFile.snapshotId());
statisticsFiles.put(statisticsFile.snapshotId(), ImmutableList.of(statisticsFile));
changes.add(new MetadataUpdate.SetStatistics(snapshotId, statisticsFile));
changes.add(new MetadataUpdate.SetStatistics(statisticsFile));
return this;
}

public Builder setStatistics(StatisticsFile statisticsFile) {
Preconditions.checkNotNull(statisticsFile, "statisticsFile is null");
statisticsFiles.put(statisticsFile.snapshotId(), ImmutableList.of(statisticsFile));
changes.add(new MetadataUpdate.SetStatistics(statisticsFile));
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,6 @@ public void testSetStatistics() {
long snapshotId = 1940541653261589030L;
MetadataUpdate expected =
new MetadataUpdate.SetStatistics(
snapshotId,
new GenericStatisticsFile(
snapshotId,
"s3://bucket/warehouse/stats.puffin",
Expand Down
6 changes: 5 additions & 1 deletion open-api/rest-catalog-open-api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1024,7 +1024,11 @@ class Term(BaseModel):

class SetStatisticsUpdate(BaseUpdate):
action: str = Field('set-statistics', const=True)
snapshot_id: int = Field(..., alias='snapshot-id')
snapshot_id: Optional[int] = Field(
None,
alias='snapshot-id',
description='This optional field is **DEPRECATED for REMOVAL** since it contains redundant information. Clients should use the `statistics.snapshot-id` field instead.',
)
statistics: StatisticsFile


Expand Down
5 changes: 4 additions & 1 deletion open-api/rest-catalog-open-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2881,7 +2881,6 @@ components:
allOf:
- $ref: '#/components/schemas/BaseUpdate'
required:
- snapshot-id
- statistics
properties:
action:
Expand All @@ -2890,6 +2889,10 @@ components:
snapshot-id:
type: integer
format: int64
deprecated: true
description:
Fokko marked this conversation as resolved.
Show resolved Hide resolved
This optional field is **DEPRECATED for REMOVAL** since it contains redundant information.
Clients should use the `statistics.snapshot-id` field instead.
statistics:
$ref: '#/components/schemas/StatisticsFile'

Expand Down