From 4becc0c6f4919f7b0ce178df23a739d400ad4880 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Mon, 29 Jun 2026 15:45:14 -0700 Subject: [PATCH 01/13] Core: Add id tracking to MetricsConfig. --- .../org/apache/iceberg/MetricsConfig.java | 128 +++++++++++------- .../java/org/apache/iceberg/MetricsModes.java | 24 +++- .../org/apache/iceberg/PropertiesUpdate.java | 2 +- .../org/apache/iceberg/TableMetadata.java | 2 +- .../iceberg/TestFileGenerationUtil.java | 4 +- .../java/org/apache/iceberg/TestMetrics.java | 24 ++-- .../org/apache/iceberg/TestMetricsConfig.java | 93 +++++++++++++ .../org/apache/iceberg/TestMetricsModes.java | 4 +- .../iceberg/data/BaseFileWriterFactory.java | 3 +- .../iceberg/data/GenericAppenderFactory.java | 9 +- .../data/GenericFileWriterFactory.java | 5 +- .../data/RegistryBasedFileWriterFactory.java | 3 +- .../flink/sink/FlinkAppenderFactory.java | 2 +- .../flink/sink/FlinkAppenderFactory.java | 2 +- .../flink/sink/FlinkAppenderFactory.java | 2 +- .../spark/source/SparkFileWriterFactory.java | 5 +- .../iceberg/spark/TestSparkTableUtil.java | 4 +- 17 files changed, 230 insertions(+), 86 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/MetricsConfig.java b/core/src/main/java/org/apache/iceberg/MetricsConfig.java index 2b55bcbeab22..a64666941e38 100644 --- a/core/src/main/java/org/apache/iceberg/MetricsConfig.java +++ b/core/src/main/java/org/apache/iceberg/MetricsConfig.java @@ -26,13 +26,13 @@ import java.io.Serializable; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Supplier; import javax.annotation.concurrent.Immutable; import org.apache.iceberg.MetricsModes.MetricsMode; import org.apache.iceberg.exceptions.ValidationException; -import org.apache.iceberg.relocated.com.google.common.base.Joiner; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.relocated.com.google.common.collect.Sets; @@ -49,12 +49,12 @@ public final class MetricsConfig implements Serializable { private static final Logger LOG = LoggerFactory.getLogger(MetricsConfig.class); - private static final Joiner DOT = Joiner.on('.'); // Disable metrics by default for wide tables to prevent excessive metadata private static final MetricsMode DEFAULT_MODE = MetricsModes.fromString(DEFAULT_WRITE_METRICS_MODE_DEFAULT); - private static final MetricsConfig DEFAULT = new MetricsConfig(ImmutableMap.of(), DEFAULT_MODE); + private static final MetricsConfig DEFAULT = + new MetricsConfig(ImmutableMap.of(), DEFAULT_MODE, ImmutableMap.of()); private static final MetricsConfig POSITION_DELETE_MODE = new MetricsConfig( ImmutableMap.of( @@ -62,14 +62,26 @@ public final class MetricsConfig implements Serializable { MetricsModes.Full.get(), MetadataColumns.DELETE_FILE_POS.name(), MetricsModes.Full.get()), - DEFAULT_MODE); + DEFAULT_MODE, + ImmutableMap.of( + MetadataColumns.DELETE_FILE_PATH.fieldId(), + MetadataColumns.DELETE_FILE_PATH.name(), + MetadataColumns.DELETE_FILE_POS.fieldId(), + MetadataColumns.DELETE_FILE_POS.name())); private final Map columnModes; private final MetricsMode defaultMode; + private Map idToName; - private MetricsConfig(Map columnModes, MetricsMode defaultMode) { + private MetricsConfig( + Map columnModes, + MetricsMode defaultMode, + Map idToName) { this.columnModes = SerializableMap.copyOf(columnModes).immutableMap(); this.defaultMode = defaultMode; + if (idToName != null) { + this.idToName = SerializableMap.copyOf(idToName).immutableMap(); + } } public static MetricsConfig getDefault() { @@ -84,13 +96,23 @@ public static MetricsConfig forPositionDelete() { * Creates a metrics config from table configuration. * * @param props table configuration - * @deprecated use {@link MetricsConfig#forTable(Table)}. Will be removed in 2.0.0 + * @deprecated use {@link MetricsConfig#forTable(Table)}. Will be removed in 1.13.0 */ @Deprecated public static MetricsConfig fromProperties(Map props) { return from(props, null, null); } + /** + * Validates metrics config properties with the given schema. + * + * @param props table properties + * @param schema table schema + */ + public static void validate(Map props, Schema schema) { + from(props, schema, null).validateReferencedColumns(schema); + } + /** * Creates a metrics config from a table. * @@ -100,32 +122,40 @@ public static MetricsConfig forTable(Table table) { return from(table.properties(), table.schema(), table.sortOrder()); } - /** - * Creates a metrics config for a position delete file. - * - * @param table an Iceberg table - * @deprecated This method is deprecated as of version 1.11.0 and will be removed in 1.12.0. - * Position deletes that include row data are no longer supported. Use {@link - * #forPositionDelete()} instead. - */ - @Deprecated - public static MetricsConfig forPositionDelete(Table table) { - ImmutableMap.Builder columnModes = ImmutableMap.builder(); + public Iterable metricsFieldIds() { + return idToName.keySet(); + } - columnModes.put(MetadataColumns.DELETE_FILE_PATH.name(), MetricsModes.Full.get()); - columnModes.put(MetadataColumns.DELETE_FILE_POS.name(), MetricsModes.Full.get()); + public MetricsMode columnMode(int id) { + String name = idToName.get(id); + if (name != null) { + return columnMode(name); + } - MetricsConfig tableConfig = forTable(table); + return defaultMode; + } - MetricsMode defaultMode = tableConfig.defaultMode; - tableConfig.columnModes.forEach( - (columnAlias, mode) -> { - String positionDeleteColumnAlias = - DOT.join(MetadataColumns.DELETE_FILE_ROW_FIELD_NAME, columnAlias); - columnModes.put(positionDeleteColumnAlias, mode); - }); + public MetricsMode columnMode(String columnAlias) { + return columnModes.getOrDefault(columnAlias, defaultMode); + } + + public void validateReferencedColumns(Schema schema) { + for (String column : columnModes.keySet()) { + Types.NestedField field = schema.findField(column); + ValidationException.check( + field != null, + "Cannot find column %s from table prop %s in schema %s", + column, + METRICS_MODE_COLUMN_CONF_PREFIX + column, + schema); - return new MetricsConfig(columnModes.build(), defaultMode); + ValidationException.check( + null == idToName || idToName.get(field.fieldId()).equals(column), + "Incorrect field name for id %s: %s (expected %s)", + field.fieldId(), + column, + idToName.get(field.fieldId())); + } } static Set limitFieldIds(Schema schema, int limit) { @@ -225,6 +255,7 @@ public Set map( */ public static MetricsConfig from(Map props, Schema schema, SortOrder order) { int maxInferredDefaultColumns = maxInferredColumnDefaults(props); + Map idToName = Maps.newHashMap(); Map columnModes = Maps.newHashMap(); // Handle user override of default mode @@ -237,13 +268,20 @@ public static MetricsConfig from(Map props, Schema schema, SortO } else if (schema == null) { defaultMode = DEFAULT_MODE; } else { - if (TypeUtil.getProjectedIds(schema).size() <= maxInferredDefaultColumns) { + Set ids = TypeUtil.getProjectedIds(schema); + if (ids.size() <= maxInferredDefaultColumns) { + for (int id : ids) { + idToName.put(id, schema.findColumnName(id)); + } + // there are less than the inferred limit (including structs), so the default is used // everywhere defaultMode = DEFAULT_MODE; } else { for (Integer id : limitFieldIds(schema, maxInferredDefaultColumns)) { - columnModes.put(schema.findColumnName(id), DEFAULT_MODE); + String name = schema.findColumnName(id); + idToName.put(id, name); + columnModes.put(name, DEFAULT_MODE); } // all other columns don't use metrics @@ -254,7 +292,14 @@ public static MetricsConfig from(Map props, Schema schema, SortO // First set sorted column with sorted column default (can be overridden by user) MetricsMode sortedColDefaultMode = sortedColumnDefaultMode(defaultMode); Set sortedCols = SortOrderUtil.orderPreservingSortedColumns(order); - sortedCols.forEach(sc -> columnModes.put(sc, sortedColDefaultMode)); + sortedCols.forEach( + name -> { + columnModes.put(name, sortedColDefaultMode); + Types.NestedField field = schema != null ? schema.findField(name) : null; + if (field != null) { + idToName.put(field.fieldId(), name); + } + }); // Handle user overrides of defaults for (String key : props.keySet()) { @@ -262,10 +307,14 @@ public static MetricsConfig from(Map props, Schema schema, SortO String columnAlias = key.replaceFirst(METRICS_MODE_COLUMN_CONF_PREFIX, ""); MetricsMode mode = parseMode(props.get(key), defaultMode, "column " + columnAlias); columnModes.put(columnAlias, mode); + Types.NestedField field = schema != null ? schema.findField(columnAlias) : null; + if (field != null) { + idToName.put(field.fieldId(), columnAlias); + } } } - return new MetricsConfig(columnModes, defaultMode); + return new MetricsConfig(columnModes, defaultMode, idToName); } /** @@ -309,19 +358,4 @@ private static MetricsMode parseMode(String modeString, MetricsMode fallback, St return fallback; } } - - public void validateReferencedColumns(Schema schema) { - for (String column : columnModes.keySet()) { - ValidationException.check( - schema.findField(column) != null, - "Invalid metrics config, could not find column %s from table prop %s in schema %s", - column, - METRICS_MODE_COLUMN_CONF_PREFIX + column, - schema); - } - } - - public MetricsMode columnMode(String columnAlias) { - return columnModes.getOrDefault(columnAlias, defaultMode); - } } diff --git a/core/src/main/java/org/apache/iceberg/MetricsModes.java b/core/src/main/java/org/apache/iceberg/MetricsModes.java index cd5d5eab1273..f5d7f5813724 100644 --- a/core/src/main/java/org/apache/iceberg/MetricsModes.java +++ b/core/src/main/java/org/apache/iceberg/MetricsModes.java @@ -59,7 +59,9 @@ public static MetricsMode fromString(String mode) { * *

Implementations must be immutable. */ - public interface MetricsMode extends Serializable {} + public interface MetricsMode extends Serializable { + boolean hasBounds(); + } /** * Under this mode, value_counts, null_value_counts, nan_value_counts, lower_bounds, upper_bounds @@ -72,6 +74,11 @@ public static None get() { return INSTANCE; } + @Override + public boolean hasBounds() { + return false; + } + @Override public String toString() { return "none"; @@ -86,6 +93,11 @@ public static Counts get() { return INSTANCE; } + @Override + public boolean hasBounds() { + return false; + } + @Override public String toString() { return "counts"; @@ -108,6 +120,11 @@ public static Truncate withLength(int length) { return new Truncate(length); } + @Override + public boolean hasBounds() { + return true; + } + public int length() { return length; } @@ -145,6 +162,11 @@ public static Full get() { return INSTANCE; } + @Override + public boolean hasBounds() { + return true; + } + @Override public String toString() { return "full"; diff --git a/core/src/main/java/org/apache/iceberg/PropertiesUpdate.java b/core/src/main/java/org/apache/iceberg/PropertiesUpdate.java index 9389aec50c0a..30efc66592c5 100644 --- a/core/src/main/java/org/apache/iceberg/PropertiesUpdate.java +++ b/core/src/main/java/org/apache/iceberg/PropertiesUpdate.java @@ -90,7 +90,7 @@ public Map apply() { // Validate the metrics if (base != null && base.schema() != null) { - MetricsConfig.fromProperties(newProperties).validateReferencedColumns(base.schema()); + MetricsConfig.validate(newProperties, base.schema()); } return newProperties; diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java index 07f6d3342727..12b2ab88ddf6 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadata.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java @@ -139,7 +139,7 @@ static TableMetadata newTableMetadata( // Validate the metrics configuration. Note: we only do this on new tables to we don't // break existing tables. - MetricsConfig.fromProperties(properties).validateReferencedColumns(schema); + MetricsConfig.validate(properties, schema); PropertyUtil.validateCommitProperties(properties); diff --git a/core/src/test/java/org/apache/iceberg/TestFileGenerationUtil.java b/core/src/test/java/org/apache/iceberg/TestFileGenerationUtil.java index f533df159893..c167bb7039fc 100644 --- a/core/src/test/java/org/apache/iceberg/TestFileGenerationUtil.java +++ b/core/src/test/java/org/apache/iceberg/TestFileGenerationUtil.java @@ -92,8 +92,8 @@ public void testBoundsWithSpecificValues() { @SuppressWarnings("deprecation") void testBoundsForAllMetricsModes(String metricsMode) { MetricsConfig metricsConfig = - MetricsConfig.fromProperties( - ImmutableMap.of(TableProperties.DEFAULT_WRITE_METRICS_MODE, metricsMode)); + MetricsConfig.from( + ImmutableMap.of(TableProperties.DEFAULT_WRITE_METRICS_MODE, metricsMode), SCHEMA, null); Metrics metrics = FileGenerationUtil.generateRandomMetrics( SCHEMA, diff --git a/core/src/test/java/org/apache/iceberg/TestMetrics.java b/core/src/test/java/org/apache/iceberg/TestMetrics.java index 874bb6d74d44..5dece00fc5d0 100644 --- a/core/src/test/java/org/apache/iceberg/TestMetrics.java +++ b/core/src/test/java/org/apache/iceberg/TestMetrics.java @@ -326,7 +326,7 @@ public void testMetricsModeForNestedStructFields() throws IOException { MetricsModes.None.get().toString(), TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "nestedStructCol.longCol", MetricsModes.Full.get().toString()); - MetricsConfig config = MetricsConfig.fromProperties(properties); + MetricsConfig config = MetricsConfig.from(properties, NESTED_SCHEMA, null); Metrics metrics = getMetrics(NESTED_SCHEMA, config, buildNestedTestRecord()); MetricsWithStats metricsWithStats = @@ -590,7 +590,8 @@ public void testNoneMetricsMode() throws IOException { Metrics metrics = getMetrics( NESTED_SCHEMA, - MetricsConfig.fromProperties(ImmutableMap.of("write.metadata.metrics.default", "none")), + MetricsConfig.from( + ImmutableMap.of("write.metadata.metrics.default", "none"), NESTED_SCHEMA, null), buildNestedTestRecord()); MetricsWithStats metricsWithStats = new MetricsWithStats(metrics, MetricsUtil.fromMetrics(NESTED_SCHEMA, metrics)); @@ -613,8 +614,8 @@ public void testCountsMetricsMode() throws IOException { Metrics metrics = getMetrics( NESTED_SCHEMA, - MetricsConfig.fromProperties( - ImmutableMap.of("write.metadata.metrics.default", "counts")), + MetricsConfig.from( + ImmutableMap.of("write.metadata.metrics.default", "counts"), NESTED_SCHEMA, null), buildNestedTestRecord()); MetricsWithStats metricsWithStats = new MetricsWithStats(metrics, MetricsUtil.fromMetrics(NESTED_SCHEMA, metrics)); @@ -638,7 +639,8 @@ public void testFullMetricsMode() throws IOException { Metrics metrics = getMetrics( NESTED_SCHEMA, - MetricsConfig.fromProperties(ImmutableMap.of("write.metadata.metrics.default", "full")), + MetricsConfig.from( + ImmutableMap.of("write.metadata.metrics.default", "full"), NESTED_SCHEMA, null), buildNestedTestRecord()); MetricsWithStats metricsWithStats = new MetricsWithStats(metrics, MetricsUtil.fromMetrics(NESTED_SCHEMA, metrics)); @@ -675,8 +677,10 @@ public void testTruncateStringMetricsMode() throws IOException { Metrics metrics = getMetrics( singleStringColSchema, - MetricsConfig.fromProperties( - ImmutableMap.of("write.metadata.metrics.default", "truncate(10)")), + MetricsConfig.from( + ImmutableMap.of("write.metadata.metrics.default", "truncate(10)"), + singleStringColSchema, + null), record); MetricsWithStats metricsWithStats = new MetricsWithStats(metrics, MetricsUtil.fromMetrics(singleStringColSchema, metrics)); @@ -702,8 +706,10 @@ public void testTruncateBinaryMetricsMode() throws IOException { Metrics metrics = getMetrics( singleBinaryColSchema, - MetricsConfig.fromProperties( - ImmutableMap.of("write.metadata.metrics.default", "truncate(5)")), + MetricsConfig.from( + ImmutableMap.of("write.metadata.metrics.default", "truncate(5)"), + singleBinaryColSchema, + null), record); MetricsWithStats metricsWithStats = new MetricsWithStats(metrics, MetricsUtil.fromMetrics(singleBinaryColSchema, metrics)); diff --git a/core/src/test/java/org/apache/iceberg/TestMetricsConfig.java b/core/src/test/java/org/apache/iceberg/TestMetricsConfig.java index d4ed6a988fe6..4eeada847d59 100644 --- a/core/src/test/java/org/apache/iceberg/TestMetricsConfig.java +++ b/core/src/test/java/org/apache/iceberg/TestMetricsConfig.java @@ -22,7 +22,9 @@ import static org.apache.iceberg.types.Types.NestedField.required; import static org.assertj.core.api.Assertions.assertThat; +import java.util.Map; import java.util.Set; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.types.Types; import org.junit.jupiter.api.Test; @@ -112,4 +114,95 @@ public void testNestedListOfMaps() { assertThat(MetricsConfig.limitFieldIds(schema, 3)).isEqualTo(Set.of(5, 3, 4)); assertThat(MetricsConfig.limitFieldIds(schema, 4)).isEqualTo(Set.of(5, 3, 4)); } + + @Test + public void testColumnModeAndFieldIdsFromColumnConfig() { + Schema schema = + new Schema( + required(1, "id", Types.IntegerType.get()), + optional(2, "data", Types.StringType.get()), + optional(3, "category", Types.StringType.get())); + + Map props = + ImmutableMap.of( + TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "id", "full", + TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "data", "none"); + + MetricsConfig config = MetricsConfig.from(props, schema, null); + + assertThat(config.metricsFieldIds()) + .as("Should track field ids for configured and defaulted columns") + .containsExactlyInAnyOrder(1, 2, 3); + + assertThat(config.columnMode(1)) + .as("Should return the configured mode for a tracked field id") + .isEqualTo(MetricsModes.Full.get()); + assertThat(config.columnMode(2)).isEqualTo(MetricsModes.None.get()); + assertThat(config.columnMode(3)) + .as("Defaulted column should use the default mode") + .isEqualTo(MetricsModes.Truncate.withLength(16)); + } + + @Test + public void testAllDefaultedColumnsTracked() { + Schema schema = + new Schema( + required(1, "id", Types.IntegerType.get()), + optional(2, "data", Types.StringType.get()), + optional(3, "category", Types.StringType.get())); + + MetricsConfig config = MetricsConfig.from(ImmutableMap.of(), schema, null); + + assertThat(config.metricsFieldIds()) + .as("Should track field ids for all columns that use the default mode") + .containsExactlyInAnyOrder(1, 2, 3); + + assertThat(config.columnMode(1)).isEqualTo(MetricsModes.Truncate.withLength(16)); + assertThat(config.columnMode(2)).isEqualTo(MetricsModes.Truncate.withLength(16)); + assertThat(config.columnMode(3)).isEqualTo(MetricsModes.Truncate.withLength(16)); + } + + @Test + public void testLimitingMetricsFieldIds() { + Schema schema = + new Schema( + required(1, "a", Types.IntegerType.get()), + required(2, "b", Types.IntegerType.get()), + required(3, "c", Types.IntegerType.get()), + required(4, "d", Types.IntegerType.get()), + required(5, "e", Types.IntegerType.get())); + + Map limitedToTwo = + ImmutableMap.of(TableProperties.METRICS_MAX_INFERRED_COLUMN_DEFAULTS, "2"); + MetricsConfig config = MetricsConfig.from(limitedToTwo, schema, null); + + // only the fields within the inferred limit are tracked, matching limitFieldIds + assertThat(config.metricsFieldIds()) + .as("Should track only the field ids within the inferred column limit") + .containsExactlyInAnyOrderElementsOf(MetricsConfig.limitFieldIds(schema, 2)) + .containsExactlyInAnyOrder(1, 2); + + // tracked fields keep metrics at the default mode + assertThat(config.columnMode(1)).isEqualTo(MetricsModes.Truncate.withLength(16)); + assertThat(config.columnMode(2)).isEqualTo(MetricsModes.Truncate.withLength(16)); + // fields past the limit are dropped from both id tracking and metrics + assertThat(config.columnMode(3)) + .as("Field past the limit should not have metrics") + .isEqualTo(MetricsModes.None.get()); + assertThat(config.columnMode(4)).isEqualTo(MetricsModes.None.get()); + assertThat(config.columnMode(5)).isEqualTo(MetricsModes.None.get()); + + // raising the limit expands both the tracked ids and the columns with metrics + Map limitedToThree = + ImmutableMap.of(TableProperties.METRICS_MAX_INFERRED_COLUMN_DEFAULTS, "3"); + MetricsConfig wider = MetricsConfig.from(limitedToThree, schema, null); + + assertThat(wider.metricsFieldIds()) + .as("Raising the limit should track more field ids") + .containsExactlyInAnyOrder(1, 2, 3); + assertThat(wider.columnMode(3)) + .as("Raising the limit should give the field metrics at the default mode") + .isEqualTo(MetricsModes.Truncate.withLength(16)); + assertThat(wider.columnMode(4)).isEqualTo(MetricsModes.None.get()); + } } diff --git a/core/src/test/java/org/apache/iceberg/TestMetricsModes.java b/core/src/test/java/org/apache/iceberg/TestMetricsModes.java index 1d03e342dfb4..f96857f8fbbf 100644 --- a/core/src/test/java/org/apache/iceberg/TestMetricsModes.java +++ b/core/src/test/java/org/apache/iceberg/TestMetricsModes.java @@ -87,7 +87,7 @@ public void testInvalidColumnModeValue() { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col", "troncate(5)"); - MetricsConfig config = MetricsConfig.fromProperties(properties); + MetricsConfig config = MetricsConfig.from(properties, null, null); assertThat(config.columnMode("col")) .as("Invalid mode should be defaulted to table default (full)") .isEqualTo(MetricsModes.Full.get()); @@ -102,7 +102,7 @@ public void testInvalidDefaultColumnModeValue() { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col", "troncate(5)"); - MetricsConfig config = MetricsConfig.fromProperties(properties); + MetricsConfig config = MetricsConfig.from(properties, null, null); assertThat(config.columnMode("col")) .as("Invalid mode should be defaulted to library default (truncate(16))") .isEqualTo(MetricsModes.Truncate.withLength(16)); diff --git a/data/src/main/java/org/apache/iceberg/data/BaseFileWriterFactory.java b/data/src/main/java/org/apache/iceberg/data/BaseFileWriterFactory.java index 444c0d0226bd..f6fbf1ca6b24 100644 --- a/data/src/main/java/org/apache/iceberg/data/BaseFileWriterFactory.java +++ b/data/src/main/java/org/apache/iceberg/data/BaseFileWriterFactory.java @@ -293,8 +293,7 @@ public PositionDeleteWriter newPositionDeleteWriter( EncryptedOutputFile file, PartitionSpec spec, StructLike partition) { EncryptionKeyMetadata keyMetadata = file.keyMetadata(); Map properties = table == null ? ImmutableMap.of() : table.properties(); - MetricsConfig metricsConfig = - table == null ? MetricsConfig.forPositionDelete() : MetricsConfig.forPositionDelete(table); + MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(); try { switch (deleteFileFormat) { diff --git a/data/src/main/java/org/apache/iceberg/data/GenericAppenderFactory.java b/data/src/main/java/org/apache/iceberg/data/GenericAppenderFactory.java index dd78b48858a2..a4c62dc3f8f6 100644 --- a/data/src/main/java/org/apache/iceberg/data/GenericAppenderFactory.java +++ b/data/src/main/java/org/apache/iceberg/data/GenericAppenderFactory.java @@ -171,7 +171,7 @@ public FileAppender newAppender(OutputFile outputFile, FileFormat fileFo public FileAppender newAppender( EncryptedOutputFile encryptedOutputFile, FileFormat fileFormat) { MetricsConfig metricsConfig = - table != null ? MetricsConfig.forTable(table) : MetricsConfig.fromProperties(config); + table != null ? MetricsConfig.forTable(table) : MetricsConfig.from(config, null, null); try { switch (fileFormat) { @@ -233,7 +233,7 @@ public EqualityDeleteWriter newEqDeleteWriter( eqDeleteRowSchema, "Equality delete row schema shouldn't be null when creating equality-delete writer"); MetricsConfig metricsConfig = - table != null ? MetricsConfig.forTable(table) : MetricsConfig.fromProperties(config); + table != null ? MetricsConfig.forTable(table) : MetricsConfig.from(config, null, null); try { switch (format) { @@ -287,10 +287,7 @@ public EqualityDeleteWriter newEqDeleteWriter( @Override public PositionDeleteWriter newPosDeleteWriter( EncryptedOutputFile file, FileFormat format, StructLike partition) { - MetricsConfig metricsConfig = - table != null - ? MetricsConfig.forPositionDelete(table) - : MetricsConfig.fromProperties(config); + MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(); try { switch (format) { diff --git a/data/src/main/java/org/apache/iceberg/data/GenericFileWriterFactory.java b/data/src/main/java/org/apache/iceberg/data/GenericFileWriterFactory.java index 1e75b9eda961..914dc3672b77 100644 --- a/data/src/main/java/org/apache/iceberg/data/GenericFileWriterFactory.java +++ b/data/src/main/java/org/apache/iceberg/data/GenericFileWriterFactory.java @@ -241,10 +241,7 @@ public PositionDeleteWriter newPositionDeleteWriter( LOG.warn( "Deprecated feature used. Position delete row schema is used to create the position delete writer."); Map properties = table == null ? ImmutableMap.of() : table.properties(); - MetricsConfig metricsConfig = - table == null - ? MetricsConfig.forPositionDelete() - : MetricsConfig.forPositionDelete(table); + MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(); try { return switch (format) { diff --git a/data/src/main/java/org/apache/iceberg/data/RegistryBasedFileWriterFactory.java b/data/src/main/java/org/apache/iceberg/data/RegistryBasedFileWriterFactory.java index 868b41f5840b..da2be7702909 100644 --- a/data/src/main/java/org/apache/iceberg/data/RegistryBasedFileWriterFactory.java +++ b/data/src/main/java/org/apache/iceberg/data/RegistryBasedFileWriterFactory.java @@ -160,8 +160,7 @@ public PositionDeleteWriter newPositionDeleteWriter( EncryptedOutputFile file, PartitionSpec spec, StructLike partition) { EncryptionKeyMetadata keyMetadata = file.keyMetadata(); Map properties = table != null ? table.properties() : ImmutableMap.of(); - MetricsConfig metricsConfig = - table != null ? MetricsConfig.forPositionDelete(table) : MetricsConfig.forPositionDelete(); + MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(); try { FileWriterBuilder, ?> builder = diff --git a/flink/v1.20/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java b/flink/v1.20/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java index 07a068391ce6..85dcc3133bd6 100644 --- a/flink/v1.20/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java +++ b/flink/v1.20/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java @@ -238,7 +238,7 @@ public EqualityDeleteWriter newEqDeleteWriter( @Override public PositionDeleteWriter newPosDeleteWriter( EncryptedOutputFile outputFile, FileFormat format, StructLike partition) { - MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(table); + MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(); try { switch (format) { case AVRO: diff --git a/flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java b/flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java index 07a068391ce6..85dcc3133bd6 100644 --- a/flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java +++ b/flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java @@ -238,7 +238,7 @@ public EqualityDeleteWriter newEqDeleteWriter( @Override public PositionDeleteWriter newPosDeleteWriter( EncryptedOutputFile outputFile, FileFormat format, StructLike partition) { - MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(table); + MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(); try { switch (format) { case AVRO: diff --git a/flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java b/flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java index 07a068391ce6..85dcc3133bd6 100644 --- a/flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java +++ b/flink/v2.1/flink/src/main/java/org/apache/iceberg/flink/sink/FlinkAppenderFactory.java @@ -238,7 +238,7 @@ public EqualityDeleteWriter newEqDeleteWriter( @Override public PositionDeleteWriter newPosDeleteWriter( EncryptedOutputFile outputFile, FileFormat format, StructLike partition) { - MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(table); + MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(); try { switch (format) { case AVRO: diff --git a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java index 39110f0b0597..5f13b8aac45b 100644 --- a/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java +++ b/spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java @@ -166,10 +166,7 @@ public PositionDeleteWriter newPositionDeleteWriter( } else { LOG.warn("Position deletes with deleted rows are deprecated and will be removed in 1.12.0."); Map properties = table == null ? ImmutableMap.of() : table.properties(); - MetricsConfig metricsConfig = - table == null - ? MetricsConfig.forPositionDelete() - : MetricsConfig.forPositionDelete(table); + MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(); try { return switch (deleteFormat) { diff --git a/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java b/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java index 93e4c8968715..b57480a61eb1 100644 --- a/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java +++ b/spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java @@ -65,7 +65,7 @@ public void testMetricsConfigKryoSerialization() throws Exception { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col2", "truncate(16)"); - MetricsConfig config = MetricsConfig.fromProperties(metricsConfig); + MetricsConfig config = MetricsConfig.from(metricsConfig, null, null); MetricsConfig deserialized = KryoHelpers.roundTripSerialize(config); assertThat(deserialized.columnMode("col1")) @@ -90,7 +90,7 @@ public void testMetricsConfigJavaSerialization() throws Exception { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col2", "truncate(16)"); - MetricsConfig config = MetricsConfig.fromProperties(metricsConfig); + MetricsConfig config = MetricsConfig.from(metricsConfig, null, null); MetricsConfig deserialized = TestHelpers.roundTripSerialize(config); assertThat(deserialized.columnMode("col1")) From 173594a86fe582a68385ff98f45ff9784ad54cce Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Tue, 30 Jun 2026 11:02:37 -0700 Subject: [PATCH 02/13] Apply spotless. --- core/src/main/java/org/apache/iceberg/MetricsConfig.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/MetricsConfig.java b/core/src/main/java/org/apache/iceberg/MetricsConfig.java index a64666941e38..3ae6ba51b287 100644 --- a/core/src/main/java/org/apache/iceberg/MetricsConfig.java +++ b/core/src/main/java/org/apache/iceberg/MetricsConfig.java @@ -26,7 +26,6 @@ import java.io.Serializable; import java.util.Iterator; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Supplier; From 42b26ccbc74a39c2801cc6480f410ebadd2f4e2e Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Tue, 30 Jun 2026 11:28:12 -0700 Subject: [PATCH 03/13] Update older Spark versions. --- .../apache/iceberg/spark/source/SparkFileWriterFactory.java | 5 +---- .../java/org/apache/iceberg/spark/TestSparkTableUtil.java | 4 ++-- .../apache/iceberg/spark/source/SparkFileWriterFactory.java | 5 +---- .../java/org/apache/iceberg/spark/TestSparkTableUtil.java | 4 ++-- 4 files changed, 6 insertions(+), 12 deletions(-) diff --git a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java index 39110f0b0597..5f13b8aac45b 100644 --- a/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java +++ b/spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java @@ -166,10 +166,7 @@ public PositionDeleteWriter newPositionDeleteWriter( } else { LOG.warn("Position deletes with deleted rows are deprecated and will be removed in 1.12.0."); Map properties = table == null ? ImmutableMap.of() : table.properties(); - MetricsConfig metricsConfig = - table == null - ? MetricsConfig.forPositionDelete() - : MetricsConfig.forPositionDelete(table); + MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(); try { return switch (deleteFormat) { diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java index 93e4c8968715..326b89ecd7b9 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java @@ -65,7 +65,7 @@ public void testMetricsConfigKryoSerialization() throws Exception { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col2", "truncate(16)"); - MetricsConfig config = MetricsConfig.fromProperties(metricsConfig); + MetricsConfig config = MetricsConfig.from(metricsConfig, null, null);; MetricsConfig deserialized = KryoHelpers.roundTripSerialize(config); assertThat(deserialized.columnMode("col1")) @@ -90,7 +90,7 @@ public void testMetricsConfigJavaSerialization() throws Exception { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col2", "truncate(16)"); - MetricsConfig config = MetricsConfig.fromProperties(metricsConfig); + MetricsConfig config = MetricsConfig.from(metricsConfig, null, null);; MetricsConfig deserialized = TestHelpers.roundTripSerialize(config); assertThat(deserialized.columnMode("col1")) diff --git a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java index 39110f0b0597..5f13b8aac45b 100644 --- a/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java +++ b/spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/source/SparkFileWriterFactory.java @@ -166,10 +166,7 @@ public PositionDeleteWriter newPositionDeleteWriter( } else { LOG.warn("Position deletes with deleted rows are deprecated and will be removed in 1.12.0."); Map properties = table == null ? ImmutableMap.of() : table.properties(); - MetricsConfig metricsConfig = - table == null - ? MetricsConfig.forPositionDelete() - : MetricsConfig.forPositionDelete(table); + MetricsConfig metricsConfig = MetricsConfig.forPositionDelete(); try { return switch (deleteFormat) { diff --git a/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java b/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java index 93e4c8968715..326b89ecd7b9 100644 --- a/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java +++ b/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java @@ -65,7 +65,7 @@ public void testMetricsConfigKryoSerialization() throws Exception { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col2", "truncate(16)"); - MetricsConfig config = MetricsConfig.fromProperties(metricsConfig); + MetricsConfig config = MetricsConfig.from(metricsConfig, null, null);; MetricsConfig deserialized = KryoHelpers.roundTripSerialize(config); assertThat(deserialized.columnMode("col1")) @@ -90,7 +90,7 @@ public void testMetricsConfigJavaSerialization() throws Exception { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col2", "truncate(16)"); - MetricsConfig config = MetricsConfig.fromProperties(metricsConfig); + MetricsConfig config = MetricsConfig.from(metricsConfig, null, null);; MetricsConfig deserialized = TestHelpers.roundTripSerialize(config); assertThat(deserialized.columnMode("col1")) From 8e1e7b2a925f05586a8caae8bce1e9c2608228e1 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Tue, 30 Jun 2026 11:31:14 -0700 Subject: [PATCH 04/13] Fixup last commit's typo. --- .../java/org/apache/iceberg/spark/TestSparkTableUtil.java | 4 ++-- .../java/org/apache/iceberg/spark/TestSparkTableUtil.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java index 326b89ecd7b9..b57480a61eb1 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java @@ -65,7 +65,7 @@ public void testMetricsConfigKryoSerialization() throws Exception { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col2", "truncate(16)"); - MetricsConfig config = MetricsConfig.from(metricsConfig, null, null);; + MetricsConfig config = MetricsConfig.from(metricsConfig, null, null); MetricsConfig deserialized = KryoHelpers.roundTripSerialize(config); assertThat(deserialized.columnMode("col1")) @@ -90,7 +90,7 @@ public void testMetricsConfigJavaSerialization() throws Exception { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col2", "truncate(16)"); - MetricsConfig config = MetricsConfig.from(metricsConfig, null, null);; + MetricsConfig config = MetricsConfig.from(metricsConfig, null, null); MetricsConfig deserialized = TestHelpers.roundTripSerialize(config); assertThat(deserialized.columnMode("col1")) diff --git a/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java b/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java index 326b89ecd7b9..b57480a61eb1 100644 --- a/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java +++ b/spark/v4.0/spark/src/test/java/org/apache/iceberg/spark/TestSparkTableUtil.java @@ -65,7 +65,7 @@ public void testMetricsConfigKryoSerialization() throws Exception { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col2", "truncate(16)"); - MetricsConfig config = MetricsConfig.from(metricsConfig, null, null);; + MetricsConfig config = MetricsConfig.from(metricsConfig, null, null); MetricsConfig deserialized = KryoHelpers.roundTripSerialize(config); assertThat(deserialized.columnMode("col1")) @@ -90,7 +90,7 @@ public void testMetricsConfigJavaSerialization() throws Exception { TableProperties.METRICS_MODE_COLUMN_CONF_PREFIX + "col2", "truncate(16)"); - MetricsConfig config = MetricsConfig.from(metricsConfig, null, null);; + MetricsConfig config = MetricsConfig.from(metricsConfig, null, null); MetricsConfig deserialized = TestHelpers.roundTripSerialize(config); assertThat(deserialized.columnMode("col1")) From ab334daff11d333c540a968cbfb2b89389e69c2c Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Tue, 30 Jun 2026 11:39:58 -0700 Subject: [PATCH 05/13] Add default implementation of hasBounds(). --- core/src/main/java/org/apache/iceberg/MetricsModes.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/MetricsModes.java b/core/src/main/java/org/apache/iceberg/MetricsModes.java index f5d7f5813724..d3f6dfa3cc24 100644 --- a/core/src/main/java/org/apache/iceberg/MetricsModes.java +++ b/core/src/main/java/org/apache/iceberg/MetricsModes.java @@ -60,7 +60,10 @@ public static MetricsMode fromString(String mode) { *

Implementations must be immutable. */ public interface MetricsMode extends Serializable { - boolean hasBounds(); + default boolean hasBounds() { + throw new UnsupportedOperationException( + "Unexpected implementation of MetricsMode without hasBounds"); + } } /** From bdcb363303d19962d2898bd25e592a3550d26a7b Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Tue, 30 Jun 2026 11:48:45 -0700 Subject: [PATCH 06/13] Update revapi to remove deprecated method. --- .palantir/revapi.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml index 80fa8f15f168..478b78dea3e3 100644 --- a/.palantir/revapi.yml +++ b/.palantir/revapi.yml @@ -564,6 +564,9 @@ acceptedBreaks: - code: "java.field.removedWithConstant" old: "field org.apache.iceberg.PartitionStatsHandler.PARTITION_FIELD_NAME" justification: "Removed deprecated functionality for partition stats" + - code: "java.method.removed" + old: "method org.apache.iceberg.MetricsConfig org.apache.iceberg.MetricsConfig::forPositionDelete(org.apache.iceberg.Table)" + justification: "Deprecated for removal in 1.12.0" - code: "java.method.removed" old: "method org.apache.iceberg.Schema org.apache.iceberg.PartitionStatsHandler::schema(org.apache.iceberg.types.Types.StructType,\ \ int)" From 306effee15ed4e4036efa2486d1150f77d962bc6 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Tue, 30 Jun 2026 13:02:19 -0700 Subject: [PATCH 07/13] Use longer error message to avoid changing another test file. --- core/src/main/java/org/apache/iceberg/MetricsConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/MetricsConfig.java b/core/src/main/java/org/apache/iceberg/MetricsConfig.java index 3ae6ba51b287..0d990803173d 100644 --- a/core/src/main/java/org/apache/iceberg/MetricsConfig.java +++ b/core/src/main/java/org/apache/iceberg/MetricsConfig.java @@ -143,7 +143,7 @@ public void validateReferencedColumns(Schema schema) { Types.NestedField field = schema.findField(column); ValidationException.check( field != null, - "Cannot find column %s from table prop %s in schema %s", + "Invalid metrics config, could not find column %s from table prop %s in schema %s", column, METRICS_MODE_COLUMN_CONF_PREFIX + column, schema); From 1cea2e1c087bfe0cfae230e4daad7ed8cb6c78d1 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Wed, 1 Jul 2026 12:34:36 -0700 Subject: [PATCH 08/13] Fix Amogh's review comments. --- .../org/apache/iceberg/MetricsConfig.java | 11 +++++----- .../apache/iceberg/io/TestWriterMetrics.java | 20 +++++-------------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/MetricsConfig.java b/core/src/main/java/org/apache/iceberg/MetricsConfig.java index 0d990803173d..8a81cf4fbf8d 100644 --- a/core/src/main/java/org/apache/iceberg/MetricsConfig.java +++ b/core/src/main/java/org/apache/iceberg/MetricsConfig.java @@ -30,6 +30,7 @@ import java.util.Set; import java.util.function.Supplier; import javax.annotation.concurrent.Immutable; +import org.apache.curator.shaded.com.google.common.base.Preconditions; import org.apache.iceberg.MetricsModes.MetricsMode; import org.apache.iceberg.exceptions.ValidationException; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -61,7 +62,7 @@ public final class MetricsConfig implements Serializable { MetricsModes.Full.get(), MetadataColumns.DELETE_FILE_POS.name(), MetricsModes.Full.get()), - DEFAULT_MODE, + MetricsModes.None.get(), ImmutableMap.of( MetadataColumns.DELETE_FILE_PATH.fieldId(), MetadataColumns.DELETE_FILE_PATH.name(), @@ -78,9 +79,7 @@ private MetricsConfig( Map idToName) { this.columnModes = SerializableMap.copyOf(columnModes).immutableMap(); this.defaultMode = defaultMode; - if (idToName != null) { - this.idToName = SerializableMap.copyOf(idToName).immutableMap(); - } + this.idToName = idToName != null ? SerializableMap.copyOf(idToName).immutableMap() : null; } public static MetricsConfig getDefault() { @@ -122,10 +121,12 @@ public static MetricsConfig forTable(Table table) { } public Iterable metricsFieldIds() { + Preconditions.checkState(idToName != null, "Cannot resolve column mode by ID: missing schema"); return idToName.keySet(); } public MetricsMode columnMode(int id) { + Preconditions.checkState(idToName != null, "Cannot resolve column mode by ID: missing schema"); String name = idToName.get(id); if (name != null) { return columnMode(name); @@ -149,7 +150,7 @@ public void validateReferencedColumns(Schema schema) { schema); ValidationException.check( - null == idToName || idToName.get(field.fieldId()).equals(column), + null == idToName || column.equals(idToName.get(field.fieldId())), "Incorrect field name for id %s: %s (expected %s)", field.fieldId(), column, diff --git a/data/src/test/java/org/apache/iceberg/io/TestWriterMetrics.java b/data/src/test/java/org/apache/iceberg/io/TestWriterMetrics.java index 4722ae98950e..310e454a47d9 100644 --- a/data/src/test/java/org/apache/iceberg/io/TestWriterMetrics.java +++ b/data/src/test/java/org/apache/iceberg/io/TestWriterMetrics.java @@ -206,8 +206,8 @@ public void testPositionDeleteMetricsCoveringMultipleDataFiles() throws IOExcept DeleteFile deleteFile = deleteWriter.toDeleteFile(); // should have NO bounds for path and position as the file covers multiple data paths - checkNotExistingRowStatistics(deleteFile.lowerBounds()); - checkNotExistingRowStatistics(deleteFile.upperBounds()); + assertThat(deleteFile.lowerBounds()).isNull(); + assertThat(deleteFile.upperBounds()).isNull(); } @TestTemplate @@ -353,21 +353,11 @@ public void testMaxColumnsWithDefaultOverride() throws IOException { } protected void checkRowStatistics(Map bounds) { - assertThat(bounds).hasSize(4); - assertThat((int) Conversions.fromByteBuffer(Types.IntegerType.get(), bounds.get(1))) - .isEqualTo(3); + assertThat(bounds).hasSize(2); + assertThat(bounds).doesNotContainKey(1); assertThat(bounds).doesNotContainKey(2); assertThat(bounds).doesNotContainKey(3); assertThat(bounds).doesNotContainKey(4); - assertThat((long) Conversions.fromByteBuffer(Types.LongType.get(), bounds.get(5))) - .isEqualTo(3L); - } - - protected void checkNotExistingRowStatistics(Map bounds) { - assertThat(bounds).hasSize(2); - assertThat((int) Conversions.fromByteBuffer(Types.IntegerType.get(), bounds.get(1))) - .isEqualTo(3); - assertThat((long) Conversions.fromByteBuffer(Types.LongType.get(), bounds.get(5))) - .isEqualTo(3L); + assertThat(bounds).doesNotContainKey(5); } } From ba1473c9380e9e0d246dd109d7d885a84eb7c97e Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Wed, 1 Jul 2026 12:39:05 -0700 Subject: [PATCH 09/13] Fix Flink tests after changes to TestWriterMetrics. --- .../apache/iceberg/flink/sink/TestFlinkWriterMetrics.java | 5 ----- .../apache/iceberg/flink/sink/TestFlinkWriterMetrics.java | 5 ----- .../apache/iceberg/flink/sink/TestFlinkWriterMetrics.java | 5 ----- 3 files changed, 15 deletions(-) diff --git a/flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java b/flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java index bcfcf5ffa502..f34ff599874b 100644 --- a/flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java +++ b/flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java @@ -64,9 +64,4 @@ public RowData toGenericRow(int value, int repeated) { protected void checkRowStatistics(Map bounds) { assertThat(bounds).hasSize(2); } - - @Override - protected void checkNotExistingRowStatistics(Map bounds) { - assertThat(bounds).isNull(); - } } diff --git a/flink/v2.0/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java b/flink/v2.0/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java index bcfcf5ffa502..f34ff599874b 100644 --- a/flink/v2.0/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java +++ b/flink/v2.0/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java @@ -64,9 +64,4 @@ public RowData toGenericRow(int value, int repeated) { protected void checkRowStatistics(Map bounds) { assertThat(bounds).hasSize(2); } - - @Override - protected void checkNotExistingRowStatistics(Map bounds) { - assertThat(bounds).isNull(); - } } diff --git a/flink/v2.1/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java b/flink/v2.1/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java index bcfcf5ffa502..f34ff599874b 100644 --- a/flink/v2.1/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java +++ b/flink/v2.1/flink/src/test/java/org/apache/iceberg/flink/sink/TestFlinkWriterMetrics.java @@ -64,9 +64,4 @@ public RowData toGenericRow(int value, int repeated) { protected void checkRowStatistics(Map bounds) { assertThat(bounds).hasSize(2); } - - @Override - protected void checkNotExistingRowStatistics(Map bounds) { - assertThat(bounds).isNull(); - } } From 19f5313e91dcf149006a623335492859d04ad6f6 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Wed, 1 Jul 2026 13:26:11 -0700 Subject: [PATCH 10/13] Fix import. --- core/src/main/java/org/apache/iceberg/MetricsConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/MetricsConfig.java b/core/src/main/java/org/apache/iceberg/MetricsConfig.java index 8a81cf4fbf8d..666600baf80f 100644 --- a/core/src/main/java/org/apache/iceberg/MetricsConfig.java +++ b/core/src/main/java/org/apache/iceberg/MetricsConfig.java @@ -30,9 +30,9 @@ import java.util.Set; import java.util.function.Supplier; import javax.annotation.concurrent.Immutable; -import org.apache.curator.shaded.com.google.common.base.Preconditions; import org.apache.iceberg.MetricsModes.MetricsMode; import org.apache.iceberg.exceptions.ValidationException; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.relocated.com.google.common.collect.Sets; From abb2417b0a3b65292efea4da32c4960b45e6fa69 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Wed, 1 Jul 2026 14:22:39 -0700 Subject: [PATCH 11/13] Fix more tests. --- .../apache/iceberg/io/TestFileWriterFactory.java | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/data/src/test/java/org/apache/iceberg/io/TestFileWriterFactory.java b/data/src/test/java/org/apache/iceberg/io/TestFileWriterFactory.java index 8354149bf495..7b4a25e918a5 100644 --- a/data/src/test/java/org/apache/iceberg/io/TestFileWriterFactory.java +++ b/data/src/test/java/org/apache/iceberg/io/TestFileWriterFactory.java @@ -301,23 +301,17 @@ public void testPositionDeleteWriterWithRow() throws IOException { } else { assertThat(referencedDataFiles).hasSize(1); assertThat(deleteFile.lowerBounds()) - .hasSize(4) + .hasSize(2) .containsKey(DELETE_FILE_PATH.fieldId()) .containsKey(DELETE_FILE_POS.fieldId()); - for (Types.NestedField column : table.schema().columns()) { - assertThat(deleteFile.lowerBounds()).containsKey(column.fieldId()); - } assertThat(deleteFile.upperBounds()) - .hasSize(4) + .hasSize(2) .containsKey(DELETE_FILE_PATH.fieldId()) .containsKey(DELETE_FILE_POS.fieldId()); - for (Types.NestedField column : table.schema().columns()) { - assertThat(deleteFile.upperBounds()).containsKey(column.fieldId()); - } // ORC also contains metrics for the deleted row struct, not just actual data fields - assertThat(deleteFile.columnSizes()).hasSizeGreaterThanOrEqualTo(4); - assertThat(deleteFile.valueCounts()).hasSizeGreaterThanOrEqualTo(2); - assertThat(deleteFile.nullValueCounts()).hasSizeGreaterThanOrEqualTo(2); + assertThat(deleteFile.columnSizes()).hasSizeGreaterThanOrEqualTo(2); + assertThat(deleteFile.valueCounts()).isNull(); + assertThat(deleteFile.nullValueCounts()).isNull(); assertThat(deleteFile.nanValueCounts()).isNull(); } From a706936968aa385dee571c976b1aeae89e977bbb Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Wed, 1 Jul 2026 14:23:10 -0700 Subject: [PATCH 12/13] Make field final. --- core/src/main/java/org/apache/iceberg/MetricsConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/MetricsConfig.java b/core/src/main/java/org/apache/iceberg/MetricsConfig.java index 666600baf80f..2522c57f131d 100644 --- a/core/src/main/java/org/apache/iceberg/MetricsConfig.java +++ b/core/src/main/java/org/apache/iceberg/MetricsConfig.java @@ -71,7 +71,7 @@ public final class MetricsConfig implements Serializable { private final Map columnModes; private final MetricsMode defaultMode; - private Map idToName; + private final Map idToName; private MetricsConfig( Map columnModes, From de8f0610f414caa1bb5357832b5bc946ffd7b3f2 Mon Sep 17 00:00:00 2001 From: Ryan Blue Date: Wed, 1 Jul 2026 14:33:54 -0700 Subject: [PATCH 13/13] Fix imports. --- .../test/java/org/apache/iceberg/io/TestFileWriterFactory.java | 1 - 1 file changed, 1 deletion(-) diff --git a/data/src/test/java/org/apache/iceberg/io/TestFileWriterFactory.java b/data/src/test/java/org/apache/iceberg/io/TestFileWriterFactory.java index 7b4a25e918a5..4bd907c3a055 100644 --- a/data/src/test/java/org/apache/iceberg/io/TestFileWriterFactory.java +++ b/data/src/test/java/org/apache/iceberg/io/TestFileWriterFactory.java @@ -53,7 +53,6 @@ import org.apache.iceberg.parquet.Parquet; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; -import org.apache.iceberg.types.Types; import org.apache.iceberg.util.CharSequenceSet; import org.apache.iceberg.util.Pair; import org.apache.iceberg.util.SerializationUtil;