Skip to content
Merged
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: 5 additions & 1 deletion api/src/main/java/org/apache/iceberg/PartitionSpec.java
Original file line number Diff line number Diff line change
Expand Up @@ -499,10 +499,14 @@ Builder add(int sourceId, int fieldId, String name, Transform<?, ?> transform) {
}

public PartitionSpec build() {
PartitionSpec spec = new PartitionSpec(schema, specId, fields, lastAssignedFieldId.get());
PartitionSpec spec = buildUnchecked();
checkCompatibility(spec, schema);
return spec;
}

PartitionSpec buildUnchecked() {
return new PartitionSpec(schema, specId, fields, lastAssignedFieldId.get());
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is needed for table replacement. Previously, building the replacement table metadata carried through partition specs and sort orders unchanged. Now that replace table goes through the same method to update the schema, setCurrentSchema, the specs and orders are now rebuilt to reference the current schema.

When replacing a table, its schema, partition spec, and sort order only need to be consistent with one another, not the older specs and sort orders. This relaxes the check so that tests that have incompatible changes don't fail.

Relaxing checks is the correct thing to do because consistency is now checked in the build method. It allows the current schema, spec, and order to be changed as long as they are consistent with one another. Older specs and orders are still kept because they may be referenced by data files in existing manifests.

}

static void checkCompatibility(PartitionSpec spec, Schema schema) {
Expand Down
15 changes: 12 additions & 3 deletions api/src/main/java/org/apache/iceberg/SortOrder.java
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,18 @@ Builder addSortField(String transformAsString, int sourceId, SortDirection direc
return this;
}

Builder addSortField(Transform<?, ?> transform, int sourceId, SortDirection direction, NullOrder nullOrder) {
fields.add(new SortField(transform, sourceId, direction, nullOrder));
return this;
}

public SortOrder build() {
SortOrder sortOrder = buildUnchecked();
checkCompatibility(sortOrder, schema);
return sortOrder;
}

SortOrder buildUnchecked() {
if (fields.isEmpty()) {
if (orderId != null && orderId != 0) {
throw new IllegalArgumentException("Unsorted order ID must be 0");
Expand All @@ -277,9 +288,7 @@ public SortOrder build() {

// default ID to 1 as 0 is reserved for unsorted order
int actualOrderId = orderId != null ? orderId : 1;
SortOrder sortOrder = new SortOrder(schema, actualOrderId, fields);
checkCompatibility(sortOrder, schema);
return sortOrder;
return new SortOrder(schema, actualOrderId, fields);
}

private Transform<?, ?> toTransform(BoundTerm<?> term) {
Expand Down
13 changes: 4 additions & 9 deletions core/src/main/java/org/apache/iceberg/BaseTransaction.java
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,10 @@ public void commitTransaction() {
}

private void commitCreateTransaction() {
// fix up the snapshot log, which should not contain intermediate snapshots
TableMetadata createMetadata = current.removeSnapshotLogEntries(intermediateSnapshotIds);

// this operation creates the table. if the commit fails, this cannot retry because another
// process has created the same table.
try {
ops.commit(null, createMetadata);
ops.commit(null, current);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Transactions no longer need to clean up the snapshot log. That cleanup is done in build based on the changes since the last commit.


} catch (RuntimeException e) {
// the commit failed and no files were committed. clean up each update.
Expand All @@ -271,9 +268,7 @@ private void commitCreateTransaction() {
}

private void commitReplaceTransaction(boolean orCreate) {
// fix up the snapshot log, which should not contain intermediate snapshots
TableMetadata replaceMetadata = current.removeSnapshotLogEntries(intermediateSnapshotIds);
Map<String, String> props = base != null ? base.properties() : replaceMetadata.properties();
Map<String, String> props = base != null ? base.properties() : current.properties();

try {
Tasks.foreach(ops)
Expand All @@ -300,7 +295,7 @@ private void commitReplaceTransaction(boolean orCreate) {
this.base = underlyingOps.current(); // just refreshed
}

underlyingOps.commit(base, replaceMetadata);
underlyingOps.commit(base, current);
});

} catch (RuntimeException e) {
Expand Down Expand Up @@ -358,7 +353,7 @@ private void commitSimpleTransaction() {
}

// fix up the snapshot log, which should not contain intermediate snapshots
underlyingOps.commit(base, current.removeSnapshotLogEntries(intermediateSnapshotIds));
underlyingOps.commit(base, current);
});

} catch (RuntimeException e) {
Expand Down
203 changes: 203 additions & 0 deletions core/src/main/java/org/apache/iceberg/MetadataUpdate.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.iceberg;

import java.io.Serializable;
import java.util.Map;
import java.util.Set;

/**
* Represents a change to table metadata.
*/
public interface MetadataUpdate extends Serializable {
class AssignUUID implements MetadataUpdate {
private final String uuid;

public AssignUUID(String uuid) {
this.uuid = uuid;
}

public String uuid() {
return uuid;
}
}

class UpgradeFormatVersion implements MetadataUpdate {
private final int formatVersion;

public UpgradeFormatVersion(int formatVersion) {
this.formatVersion = formatVersion;
}

public int formatVersion() {
return formatVersion;
}
}

class AddSchema implements MetadataUpdate {
private final Schema schema;
private final int lastColumnId;

public AddSchema(Schema schema, int lastColumnId) {
this.schema = schema;
this.lastColumnId = lastColumnId;
}

public Schema schema() {
return schema;
}

public int lastColumnId() {
return lastColumnId;
}
}

class SetCurrentSchema implements MetadataUpdate {
private final int schemaId;

public SetCurrentSchema(int schemaId) {
this.schemaId = schemaId;
}

public int schemaId() {
return schemaId;
}
}

class AddPartitionSpec implements MetadataUpdate {
private final PartitionSpec spec;

public AddPartitionSpec(PartitionSpec spec) {
this.spec = spec;
}

public PartitionSpec spec() {
return spec;
}
}

class SetDefaultPartitionSpec implements MetadataUpdate {
private final int specId;

public SetDefaultPartitionSpec(int schemaId) {
this.specId = schemaId;
}

public int specId() {
return specId;
}
}

class AddSortOrder implements MetadataUpdate {
private final SortOrder sortOrder;

public AddSortOrder(SortOrder sortOrder) {
this.sortOrder = sortOrder;
}

public SortOrder spec() {
return sortOrder;
}
}

class SetDefaultSortOrder implements MetadataUpdate {
private final int sortOrderId;

public SetDefaultSortOrder(int sortOrderId) {
this.sortOrderId = sortOrderId;
}

public int sortOrderId() {
return sortOrderId;
}
}

class AddSnapshot implements MetadataUpdate {
private final Snapshot snapshot;

public AddSnapshot(Snapshot snapshot) {
this.snapshot = snapshot;
}

public Snapshot snapshot() {
return snapshot;
}
}

class RemoveSnapshot implements MetadataUpdate {
private final long snapshotId;

public RemoveSnapshot(long snapshotId) {
this.snapshotId = snapshotId;
}

public long snapshotId() {
return snapshotId;
}
}

class SetCurrentSnapshot implements MetadataUpdate {
private final Long snapshotId;

public SetCurrentSnapshot(Long snapshotId) {
this.snapshotId = snapshotId;
}

public Long snapshotId() {
return snapshotId;
}
}

class SetProperties implements MetadataUpdate {
private final Map<String, String> updated;

public SetProperties(Map<String, String> updated) {
this.updated = updated;
}

public Map<String, String> updated() {
return updated;
}
}

class RemoveProperties implements MetadataUpdate {
private final Set<String> removed;

public RemoveProperties(Set<String> removed) {
this.removed = removed;
}

public Set<String> removed() {
return removed;
}
}

class SetLocation implements MetadataUpdate {
private final String location;

public SetLocation(String location) {
this.location = location;
}

public String location() {
return location;
}
}
}
Loading