This is an automated email from the ASF dual-hosted git repository.
diqiu50 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 8a2fbc6348 [#10986][followup] fix(iceberg): Add format version into
tableBuilder (#11031)
8a2fbc6348 is described below
commit 8a2fbc6348f2f216e0a0c854b119c37f03b40d79
Author: roryqi <[email protected]>
AuthorDate: Mon May 11 21:55:16 2026 +0800
[#10986][followup] fix(iceberg): Add format version into tableBuilder
(#11031)
### What changes were proposed in this pull request?
Add format version into tableBuilder
### Why are the changes needed?
TableMetadata properties don't contain the field `format-version`
Fix: #10986
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added UT.
---
.../iceberg/service/CatalogWrapperForREST.java | 2 ++
.../iceberg/service/TestCatalogWrapperForREST.java | 37 +++++++++++++++++++---
2 files changed, 34 insertions(+), 5 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
index bc0e456410..c5d80d80a3 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/CatalogWrapperForREST.java
@@ -97,6 +97,7 @@ import
org.apache.iceberg.rest.responses.PlanTableScanResponse;
/** Process Iceberg REST specific operations, like credential vending. */
public class CatalogWrapperForREST extends IcebergCatalogWrapper {
+ private static final String FORMAT_VERSION = "format-version";
private final CatalogCredentialManager catalogCredentialManager;
private volatile Map<String, String> catalogConfigToClients;
@@ -769,6 +770,7 @@ public class CatalogWrapperForREST extends
IcebergCatalogWrapper {
tableBuilder.withPartitionSpec(changedTableMeta.spec());
tableBuilder.withSortOrder(changedTableMeta.sortOrder());
tableBuilder.withLocation(changedTableMeta.location());
+ tableBuilder.withProperty(FORMAT_VERSION,
String.valueOf(changedTableMeta.formatVersion()));
tableBuilder.withProperties(changedTableMeta.properties());
Transaction transaction = tableBuilder.createOrReplaceTransaction();
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestCatalogWrapperForREST.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestCatalogWrapperForREST.java
index 88acbec5f7..e3a101d6e2 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestCatalogWrapperForREST.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestCatalogWrapperForREST.java
@@ -376,6 +376,7 @@ public class TestCatalogWrapperForREST {
when(tableBuilder.withPartitionSpec(any())).thenReturn(tableBuilder);
when(tableBuilder.withSortOrder(any())).thenReturn(tableBuilder);
when(tableBuilder.withLocation(any())).thenReturn(tableBuilder);
+ when(tableBuilder.withProperty(anyString(),
anyString())).thenReturn(tableBuilder);
when(tableBuilder.withProperties(any())).thenReturn(tableBuilder);
when(tableBuilder.createOrReplaceTransaction()).thenReturn(baseTransaction);
when(baseTransaction.underlyingOps()).thenReturn(ops);
@@ -401,23 +402,26 @@ public class TestCatalogWrapperForREST {
CatalogWrapperForREST wrapper = new StaticCatalogWrapperForREST("test",
config, catalog);
Schema schema = new Schema(Types.NestedField.required(1, "id",
Types.IntegerType.get()));
+ Optional<Integer> upgradeFormat = Optional.of(3);
UpdateTableRequest request =
new UpdateTableRequest(
List.of(new UpdateRequirement.AssertTableDoesNotExist()),
- stagedCreateMetadataUpdates(schema, Optional.of(3)));
+ stagedCreateMetadataUpdates(schema, upgradeFormat));
Assertions.assertDoesNotThrow(
() -> wrapper.updateTable(TableIdentifier.of("db", "tbl"), request));
verify(tableBuilder).withPartitionSpec(any());
verify(tableBuilder).withSortOrder(any());
+ verify(tableBuilder)
+ .withProperty(
+ "format-version",
expectedFormatVersionStringAfterStagedUpdates(schema, upgradeFormat));
verify(tableBuilder).withProperties(any());
- verify(tableBuilder, never()).withProperty(anyString(), anyString());
verify(tableBuilder).createOrReplaceTransaction();
}
@Test
- void testStagedCreateOmitsFormatWithProperty() {
+ void testStagedCreateSetsFormatVersionWhenNoUpgradeFormatUpdate() {
RESTCatalog catalog = mock(RESTCatalog.class);
Catalog.TableBuilder tableBuilder = mock(Catalog.TableBuilder.class);
BaseTransaction baseTransaction = mock(BaseTransaction.class);
@@ -427,6 +431,7 @@ public class TestCatalogWrapperForREST {
when(tableBuilder.withPartitionSpec(any())).thenReturn(tableBuilder);
when(tableBuilder.withSortOrder(any())).thenReturn(tableBuilder);
when(tableBuilder.withLocation(any())).thenReturn(tableBuilder);
+ when(tableBuilder.withProperty(anyString(),
anyString())).thenReturn(tableBuilder);
when(tableBuilder.withProperties(any())).thenReturn(tableBuilder);
when(tableBuilder.createOrReplaceTransaction()).thenReturn(baseTransaction);
when(baseTransaction.underlyingOps()).thenReturn(ops);
@@ -452,15 +457,37 @@ public class TestCatalogWrapperForREST {
CatalogWrapperForREST wrapper = new StaticCatalogWrapperForREST("test",
config, catalog);
Schema schema = new Schema(Types.NestedField.required(1, "id",
Types.IntegerType.get()));
+ Optional<Integer> noExplicitUpgrade = Optional.empty();
UpdateTableRequest request =
new UpdateTableRequest(
List.of(new UpdateRequirement.AssertTableDoesNotExist()),
- stagedCreateMetadataUpdates(schema, Optional.empty()));
+ stagedCreateMetadataUpdates(schema, noExplicitUpgrade));
Assertions.assertDoesNotThrow(
() -> wrapper.updateTable(TableIdentifier.of("db", "tbl"), request));
- verify(tableBuilder, never()).withProperty(anyString(), anyString());
+ verify(tableBuilder)
+ .withProperty(
+ "format-version",
+ expectedFormatVersionStringAfterStagedUpdates(schema,
noExplicitUpgrade));
+ }
+
+ /**
+ * Same derivation as {@link CatalogWrapperForREST#tableUpdateInternal} for
staged create: replay
+ * metadata updates and read {@link TableMetadata#formatVersion()}.
+ */
+ private static String expectedFormatVersionStringAfterStagedUpdates(
+ Schema schema, Optional<Integer> formatVersionForUpgrade) {
+ List<MetadataUpdate> updates = stagedCreateMetadataUpdates(schema,
formatVersionForUpgrade);
+ Optional<Integer> formatVersion =
+ updates.stream()
+ .filter(update -> update instanceof
MetadataUpdate.UpgradeFormatVersion)
+ .map(update -> ((MetadataUpdate.UpgradeFormatVersion)
update).formatVersion())
+ .findFirst();
+ TableMetadata.Builder changedMetadata =
+
formatVersion.map(TableMetadata::buildFromEmpty).orElse(TableMetadata.buildFromEmpty());
+ updates.forEach(update -> update.applyTo(changedMetadata));
+ return String.valueOf(changedMetadata.build().formatVersion());
}
/**
