From 063b7f55711bb3a84481c34876ef25b9ab754a0f Mon Sep 17 00:00:00 2001 From: Honah J Date: Thu, 5 Dec 2024 15:09:17 -0800 Subject: [PATCH 01/15] Add row-lineage fields and next-row-id Parameterize TestTableMetadata --- .../org/apache/iceberg/TableMetadata.java | 30 ++ .../apache/iceberg/TableMetadataParser.java | 26 ++ .../org/apache/iceberg/TestTableMetadata.java | 279 +++++++++++------- .../TestTableMetadataSerialization.java | 2 + 4 files changed, 238 insertions(+), 99 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java index 0e323bca1c97..9f0488366ebc 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadata.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java @@ -262,6 +262,9 @@ public String toString() { private volatile Map snapshotsById; private volatile Map refs; private volatile boolean snapshotsLoaded; + private final boolean rowLineageEnabled; + // TODO: may need to use boxing to allow null value + private final long nextRowId; @SuppressWarnings("checkstyle:CyclomaticComplexity") TableMetadata( @@ -288,6 +291,8 @@ public String toString() { Map refs, List statisticsFiles, List partitionStatisticsFiles, + boolean rowLineageEnabled, + long nextRowId, List changes) { Preconditions.checkArgument( specs != null && !specs.isEmpty(), "Partition specs cannot be null or empty"); @@ -307,6 +312,13 @@ public String toString() { Preconditions.checkArgument( metadataFileLocation == null || changes.isEmpty(), "Cannot create TableMetadata with a metadata location and changes"); + Preconditions.checkArgument( + !rowLineageEnabled || formatVersion == 3, + "Row lineage is only supported in v3 (current version v%s)", + formatVersion); + Preconditions.checkArgument( + !rowLineageEnabled || nextRowId >= 0, + "Next row id is required when row lineage is enabled"); this.metadataFileLocation = metadataFileLocation; this.formatVersion = formatVersion; @@ -340,6 +352,8 @@ public String toString() { this.refs = validateRefs(currentSnapshotId, refs, snapshotsById); this.statisticsFiles = ImmutableList.copyOf(statisticsFiles); this.partitionStatisticsFiles = ImmutableList.copyOf(partitionStatisticsFiles); + this.rowLineageEnabled = rowLineageEnabled; + this.nextRowId = nextRowId; HistoryEntry last = null; for (HistoryEntry logEntry : snapshotLog) { @@ -555,6 +569,14 @@ public List previousFiles() { return previousFiles; } + public boolean rowLinageEnabled() { + return rowLineageEnabled; + } + + public long nextRowId() { + return nextRowId; + } + public List changes() { return changes; } @@ -890,6 +912,8 @@ public static class Builder { private final Map> statisticsFiles; private final Map> partitionStatisticsFiles; private boolean suppressHistoricalSnapshots = false; + private boolean rowLinageEnabled; + private long nextRowId; // change tracking private final List changes; @@ -936,6 +960,8 @@ private Builder(int formatVersion) { this.schemasById = Maps.newHashMap(); this.specsById = Maps.newHashMap(); this.sortOrdersById = Maps.newHashMap(); + this.rowLinageEnabled = false; + this.nextRowId = -1L; } private Builder(TableMetadata base) { @@ -958,6 +984,8 @@ private Builder(TableMetadata base) { this.snapshots = Lists.newArrayList(base.snapshots()); this.changes = Lists.newArrayList(base.changes); this.startingChangeCount = changes.size(); + this.rowLinageEnabled = base.rowLineageEnabled; + this.nextRowId = base.nextRowId; this.snapshotLog = Lists.newArrayList(base.snapshotLog); this.previousFileLocation = base.metadataFileLocation; @@ -1500,6 +1528,8 @@ public TableMetadata build() { partitionStatisticsFiles.values().stream() .flatMap(List::stream) .collect(Collectors.toList()), + rowLinageEnabled, + nextRowId, discardChanges ? ImmutableList.of() : ImmutableList.copyOf(changes)); } diff --git a/core/src/main/java/org/apache/iceberg/TableMetadataParser.java b/core/src/main/java/org/apache/iceberg/TableMetadataParser.java index d7f2b29be75a..de15798c54c1 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadataParser.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadataParser.java @@ -110,6 +110,8 @@ private TableMetadataParser() {} static final String METADATA_LOG = "metadata-log"; static final String STATISTICS = "statistics"; static final String PARTITION_STATISTICS = "partition-statistics"; + static final String ROW_LINEAGE = "row-lineage"; + static final String NEXT_ROW_ID = "next-row-id"; public static void overwrite(TableMetadata metadata, OutputFile outputFile) { internalWrite(metadata, outputFile, true); @@ -240,6 +242,13 @@ public static void toJson(TableMetadata metadata, JsonGenerator generator) throw } generator.writeEndArray(); + if (metadata.formatVersion() == 3) { + generator.writeBooleanField(ROW_LINEAGE, metadata.rowLinageEnabled()); + if (metadata.nextRowId() > -1L) { + generator.writeNumberField(NEXT_ROW_ID, metadata.nextRowId()); + } + } + generator.writeArrayFieldStart(SNAPSHOT_LOG); for (HistoryEntry logEntry : metadata.snapshotLog()) { generator.writeStartObject(); @@ -352,6 +361,7 @@ public static TableMetadata fromJson(String metadataLocation, JsonNode node) { ImmutableList.Builder builder = ImmutableList.builder(); for (JsonNode schemaNode : schemaArray) { Schema current = SchemaParser.fromJson(schemaNode); + Schema.checkCompatibility(current, formatVersion); if (current.schemaId() == currentSchemaId) { schema = current; } @@ -372,6 +382,7 @@ public static TableMetadata fromJson(String metadataLocation, JsonNode node) { formatVersion == 1, "%s must exist in format v%s", SCHEMAS, formatVersion); schema = SchemaParser.fromJson(JsonUtil.get(SCHEMA, node)); + Schema.checkCompatibility(schema, formatVersion); currentSchemaId = schema.schemaId(); schemas = ImmutableList.of(schema); } @@ -521,6 +532,19 @@ public static TableMetadata fromJson(String metadataLocation, JsonNode node) { } } + boolean rowLineageEnabled = false; + // TODO: do we want to be more strict when parsing? e.g. raise if see row lineage in v2 + if (formatVersion == 3 && node.hasNonNull(ROW_LINEAGE)) { + rowLineageEnabled = JsonUtil.getBool(ROW_LINEAGE, node); + } + Long nextRowId = JsonUtil.getLongOrNull(NEXT_ROW_ID, node); + Preconditions.checkArgument( + !rowLineageEnabled || nextRowId != null, + "Next row must be set when row lineage is enabled"); + if (nextRowId == null) { + nextRowId = -1L; + } + return new TableMetadata( metadataLocation, formatVersion, @@ -545,6 +569,8 @@ public static TableMetadata fromJson(String metadataLocation, JsonNode node) { refs, statisticsFiles, partitionStatisticsFiles, + rowLineageEnabled, + nextRowId, ImmutableList.of() /* no changes from the file */); } diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index c9a8eb75a986..80f4405bc2cc 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -32,6 +32,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.entry; +import static org.assertj.core.api.Assumptions.assumeThat; import static org.junit.jupiter.params.provider.Arguments.arguments; import com.fasterxml.jackson.core.JsonGenerator; @@ -96,9 +97,18 @@ public class TestTableMetadata { public TableOperations ops = new LocalTableOperations(temp); - @Test + + private static Stream formatVersionsProvider() { + return Stream.of(1, 2, 3); + } + + + @ParameterizedTest + @MethodSource("formatVersionsProvider") @SuppressWarnings("MethodLength") - public void testJsonConversion() throws Exception { + public void testJsonConversion(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); String manifestList = @@ -162,7 +172,7 @@ public void testJsonConversion() throws Exception { TableMetadata expected = new TableMetadata( null, - 2, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, SEQ_NO, @@ -184,6 +194,8 @@ public void testJsonConversion() throws Exception { refs, statisticsFiles, partitionStatisticsFiles, + false, + 0L, ImmutableList.of()); String asJson = TableMetadataParser.toJson(expected); @@ -273,6 +285,8 @@ public void testBackwardCompat() throws Exception { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of()); String asJson = toJsonWithoutSpecAndSchemaList(expected); @@ -314,8 +328,11 @@ public void testBackwardCompat() throws Exception { assertThat(metadata.snapshot(previousSnapshotId).schemaId()).isNull(); } - @Test - public void testInvalidMainBranch() throws IOException { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testInvalidMainBranch(int formatVersion) throws IOException { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); String manifestList = @@ -359,7 +376,7 @@ public void testInvalidMainBranch() throws IOException { () -> new TableMetadata( null, - 2, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, SEQ_NO, @@ -381,13 +398,18 @@ public void testInvalidMainBranch() throws IOException { refs, ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Current snapshot ID does not match main branch"); } - @Test - public void testMainWithoutCurrent() throws IOException { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testMainWithoutCurrent(int formatVersion) throws IOException { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + long snapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); String manifestList = @@ -404,7 +426,7 @@ public void testMainWithoutCurrent() throws IOException { () -> new TableMetadata( null, - 2, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, SEQ_NO, @@ -426,13 +448,18 @@ public void testMainWithoutCurrent() throws IOException { refs, ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Current snapshot is not set, but main branch exists"); } - @Test - public void testBranchSnapshotMissing() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testBranchSnapshotMissing(int formatVersion) { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + long snapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); Schema schema = new Schema(6, Types.NestedField.required(10, "x", Types.StringType.get())); @@ -444,7 +471,7 @@ public void testBranchSnapshotMissing() { () -> new TableMetadata( null, - 2, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, SEQ_NO, @@ -466,6 +493,8 @@ public void testBranchSnapshotMissing() { refs, ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessageEndingWith("does not exist in the existing snapshots list"); @@ -513,8 +542,9 @@ private static String toJsonWithoutSpecAndSchemaList(TableMetadata metadata) { return writer.toString(); } - @Test - public void testJsonWithPreviousMetadataLog() throws Exception { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); String manifestList = @@ -548,7 +578,7 @@ public void testJsonWithPreviousMetadataLog() throws Exception { TableMetadata base = new TableMetadata( null, - 1, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, 0, @@ -570,6 +600,8 @@ public void testJsonWithPreviousMetadataLog() throws Exception { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of()); String asJson = TableMetadataParser.toJson(base); @@ -578,8 +610,9 @@ public void testJsonWithPreviousMetadataLog() throws Exception { assertThat(metadataFromJson.previousFiles()).isEqualTo(previousMetadataLog); } - @Test - public void testAddPreviousMetadataRemoveNone() throws IOException { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); String manifestList = @@ -624,7 +657,7 @@ public void testAddPreviousMetadataRemoveNone() throws IOException { TableMetadata base = new TableMetadata( latestPreviousMetadata.file(), - 1, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, 0, @@ -646,6 +679,8 @@ public void testAddPreviousMetadataRemoveNone() throws IOException { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of()); previousMetadataLog.add(latestPreviousMetadata); @@ -660,8 +695,9 @@ public void testAddPreviousMetadataRemoveNone() throws IOException { assertThat(removedPreviousMetadata).isEmpty(); } - @Test - public void testAddPreviousMetadataRemoveOne() throws IOException { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); String manifestList = @@ -715,7 +751,7 @@ public void testAddPreviousMetadataRemoveOne() throws IOException { TableMetadata base = new TableMetadata( latestPreviousMetadata.file(), - 1, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, 0, @@ -737,6 +773,8 @@ public void testAddPreviousMetadataRemoveOne() throws IOException { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of()); previousMetadataLog.add(latestPreviousMetadata); @@ -755,8 +793,9 @@ public void testAddPreviousMetadataRemoveOne() throws IOException { .isEqualTo(previousMetadataLog.subList(0, 1)); } - @Test - public void testAddPreviousMetadataRemoveMultiple() throws IOException { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); String manifestList = @@ -810,7 +849,7 @@ public void testAddPreviousMetadataRemoveMultiple() throws IOException { TableMetadata base = new TableMetadata( latestPreviousMetadata.file(), - 1, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, 0, @@ -832,6 +871,8 @@ public void testAddPreviousMetadataRemoveMultiple() throws IOException { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of()); previousMetadataLog.add(latestPreviousMetadata); @@ -850,13 +891,16 @@ public void testAddPreviousMetadataRemoveMultiple() throws IOException { .isEqualTo(previousMetadataLog.subList(0, 4)); } - @Test - public void testV2UUIDValidation() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testV2UUIDValidation(int formatVersion) { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + assertThatThrownBy( () -> new TableMetadata( null, - 2, + formatVersion, null, TEST_LOCATION, SEQ_NO, @@ -878,9 +922,11 @@ public void testV2UUIDValidation() { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of())) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("UUID is required in format v2"); + .hasMessage(String.format("UUID is required in format v%s", formatVersion)); } @Test @@ -913,6 +959,8 @@ public void testVersionValidation() { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessage( @@ -959,6 +1007,8 @@ public void testVersionValidation() { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), + false, + -1L, ImmutableList.of())) .isNotNull(); @@ -973,6 +1023,7 @@ public void testVersionValidation() { .isNotNull(); } + // TODO: add TableMetadataV3Valid.json @Test public void testParserVersionValidation() throws Exception { String supportedVersion1 = readTableMetadataInputFile("TableMetadataV1Valid.json"); @@ -989,6 +1040,7 @@ public void testParserVersionValidation() throws Exception { .hasMessageStartingWith("Cannot read unsupported version"); } + // TODO: add TableMetadataV3Valid.json @Test public void testParserV2PartitionSpecsValidation() throws Exception { String unsupportedVersion = @@ -998,6 +1050,7 @@ public void testParserV2PartitionSpecsValidation() throws Exception { .hasMessage("partition-specs must exist in format v2"); } + // TODO: add TableMetadataV3Valid.json @Test public void testParserV2LastAssignedFieldIdValidation() throws Exception { String unsupportedVersion = @@ -1007,6 +1060,7 @@ public void testParserV2LastAssignedFieldIdValidation() throws Exception { .hasMessage("last-partition-id must exist in format v2"); } + // TODO: add TableMetadataV3MissingSortOrder.json @Test public void testParserV2SortOrderValidation() throws Exception { String unsupportedVersion = readTableMetadataInputFile("TableMetadataV2MissingSortOrder.json"); @@ -1015,6 +1069,7 @@ public void testParserV2SortOrderValidation() throws Exception { .hasMessage("sort-orders must exist in format v2"); } + // TODO: add TableMetadataV3CurrentSchemaNotFound.json @Test public void testParserV2CurrentSchemaIdValidation() throws Exception { String unsupported = readTableMetadataInputFile("TableMetadataV2CurrentSchemaNotFound.json"); @@ -1023,6 +1078,7 @@ public void testParserV2CurrentSchemaIdValidation() throws Exception { .hasMessage("Cannot find schema with current-schema-id=2 from schemas"); } + // TODO: add TableMetadataV3Valid.json @Test public void testParserV2SchemasValidation() throws Exception { String unsupported = readTableMetadataInputFile("TableMetadataV2MissingSchemas.json"); @@ -1036,8 +1092,9 @@ private String readTableMetadataInputFile(String fileName) throws Exception { return String.join("", java.nio.file.Files.readAllLines(path)); } - @Test - public void testNewTableMetadataReassignmentAllIds() throws Exception { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testNewTableMetadataReassignmentAllIds(int formatVersion) throws Exception { Schema schema = new Schema( Types.NestedField.required(3, "x", Types.LongType.get()), @@ -1052,7 +1109,7 @@ public void testNewTableMetadataReassignmentAllIds() throws Exception { .build(); String location = "file://tmp/db/table"; TableMetadata metadata = - TableMetadata.newTableMetadata(schema, spec, location, ImmutableMap.of()); + TableMetadata.newTableMetadata(schema, spec, SortOrder.unsorted(), location, ImmutableMap.of(), formatVersion); // newTableMetadata should reassign column ids and partition field ids. PartitionSpec expected = @@ -1126,63 +1183,70 @@ public void testBuildReplacementForV1Table() { .isEqualTo(expected); } - @Test - public void testBuildReplacementForV2Table() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testBuildReplacementForV2AndV3Table(int formatVersion) { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + Schema schema = - new Schema( - Types.NestedField.required(1, "x", Types.LongType.get()), - Types.NestedField.required(2, "y", Types.LongType.get())); + new Schema( + Types.NestedField.required(1, "x", Types.LongType.get()), + Types.NestedField.required(2, "y", Types.LongType.get())); PartitionSpec spec = - PartitionSpec.builderFor(schema).withSpecId(0).identity("x").identity("y").build(); + PartitionSpec.builderFor(schema).withSpecId(0).identity("x").identity("y").build(); String location = "file://tmp/db/table"; TableMetadata metadata = - TableMetadata.newTableMetadata( - schema, spec, SortOrder.unsorted(), location, ImmutableMap.of(), 2); + TableMetadata.newTableMetadata( + schema, spec, SortOrder.unsorted(), location, ImmutableMap.of(), formatVersion); assertThat(metadata.spec()).isEqualTo(spec); Schema updatedSchema = - new Schema( - Types.NestedField.required(1, "x", Types.LongType.get()), - Types.NestedField.required(2, "z", Types.StringType.get())); + new Schema( + Types.NestedField.required(1, "x", Types.LongType.get()), + Types.NestedField.required(2, "z", Types.StringType.get())); PartitionSpec updatedSpec = - PartitionSpec.builderFor(updatedSchema).withSpecId(0).bucket("z", 8).identity("x").build(); + PartitionSpec.builderFor(updatedSchema).withSpecId(0).bucket("z", 8).identity("x").build(); TableMetadata updated = - metadata.buildReplacement( - updatedSchema, updatedSpec, SortOrder.unsorted(), location, ImmutableMap.of()); + metadata.buildReplacement( + updatedSchema, updatedSpec, SortOrder.unsorted(), location, ImmutableMap.of()); PartitionSpec expected = - PartitionSpec.builderFor(updated.schema()) - .withSpecId(1) - .add(3, 1002, "z_bucket", Transforms.bucket(8)) - .add(1, 1000, "x", Transforms.identity()) - .build(); + PartitionSpec.builderFor(updated.schema()) + .withSpecId(1) + .add(3, 1002, "z_bucket", Transforms.bucket(8)) + .add(1, 1000, "x", Transforms.identity()) + .build(); assertThat(updated.spec()) - .as( - "Should reassign the partition field IDs and reuse any existing IDs for equivalent fields") - .isEqualTo(expected); + .as( + "Should reassign the partition field IDs and reuse any existing IDs for equivalent fields") + .isEqualTo(expected); } - @Test - public void testSortOrder() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testSortOrder(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of()); + schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of("format-version", String.valueOf(formatVersion))); + assertThat(meta.formatVersion()).isEqualTo(formatVersion); assertThat(meta.sortOrder().isUnsorted()).isTrue(); assertThat(meta.replaceSortOrder(SortOrder.unsorted())) .as("Should detect identical unsorted order") .isSameAs(meta); } - @Test - public void testUpdateSortOrder() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testUpdateSortOrder(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); SortOrder order = SortOrder.builderFor(schema).asc("x").build(); TableMetadata sortedByX = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), order, null, ImmutableMap.of()); + schema, PartitionSpec.unpartitioned(), order, null, ImmutableMap.of("format-version", String.valueOf(formatVersion))); + assertThat(sortedByX.formatVersion()).isEqualTo(formatVersion); assertThat(sortedByX.sortOrders()).hasSize(1); assertThat(sortedByX.sortOrder().orderId()).isEqualTo(1); assertThat(sortedByX.sortOrder().fields()).hasSize(1); @@ -1213,23 +1277,25 @@ public void testUpdateSortOrder() { assertThat(sortedByX.sortOrder().fields().get(0).nullOrder()).isEqualTo(NullOrder.NULLS_FIRST); } - @Test - public void testStatistics() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of()); + schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); assertThat(meta.statisticsFiles()).as("Should default to no statistics files").isEmpty(); } - @Test - public void testSetStatistics() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testSetStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of()); + schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); TableMetadata withStatistics = TableMetadata.buildFrom(meta) @@ -1262,14 +1328,15 @@ public void testSetStatistics() { assertThat(statisticsFile.path()).isEqualTo("/some/path/to/stats/file2"); } - @Test - public void testRemoveStatistics() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testRemoveStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); TableMetadata meta = TableMetadata.buildFrom( TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of())) + schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion)) .setStatistics( 43, new GenericStatisticsFile( @@ -1294,25 +1361,27 @@ public void testRemoveStatistics() { assertThat(statisticsFile.path()).isEqualTo("/some/path/to/stats/file2"); } - @Test - public void testPartitionStatistics() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testPartitionStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of()); + schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); assertThat(meta.partitionStatisticsFiles()) .as("Should default to no partition statistics files") .isEmpty(); } - @Test - public void testSetPartitionStatistics() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testSetPartitionStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of()); + schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); TableMetadata withPartitionStatistics = TableMetadata.buildFrom(meta) @@ -1355,14 +1424,15 @@ public void testSetPartitionStatistics() { assertThat(partitionStatisticsFile.fileSizeInBytes()).isEqualTo(48L); } - @Test - public void testRemovePartitionStatistics() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testRemovePartitionStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); TableMetadata meta = TableMetadata.buildFrom( TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of())) + schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion)) .setPartitionStatistics( ImmutableGenericPartitionStatisticsFile.builder() .snapshotId(43) @@ -1395,6 +1465,7 @@ public void testRemovePartitionStatistics() { assertThat(partitionStatisticsFile.fileSizeInBytes()).isEqualTo(49L); } + // TODO: add V3 @Test public void testParseSchemaIdentifierFields() throws Exception { String data = readTableMetadataInputFile("TableMetadataV2Valid.json"); @@ -1403,6 +1474,7 @@ public void testParseSchemaIdentifierFields() throws Exception { assertThat(parsed.schemasById().get(1).identifierFieldIds()).containsExactly(1, 2); } + // TODO: add V3 @Test public void testParseMinimal() throws Exception { String data = readTableMetadataInputFile("TableMetadataV2ValidMinimal.json"); @@ -1413,13 +1485,14 @@ public void testParseMinimal() throws Exception { assertThat(parsed.previousFiles()).isEmpty(); } - @Test - public void testUpdateSchemaIdentifierFields() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testUpdateSchemaIdentifierFields(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of()); + schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); Schema newSchema = new Schema( @@ -1430,13 +1503,14 @@ public void testUpdateSchemaIdentifierFields() { assertThat(newMeta.schema().identifierFieldIds()).containsExactly(1); } - @Test - public void testUpdateSchema() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testUpdateSchema(int formatVersion) { Schema schema = new Schema(0, Types.NestedField.required(1, "y", Types.LongType.get(), "comment")); TableMetadata freshTable = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of()); + schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); assertThat(freshTable.currentSchemaId()).isEqualTo(TableMetadata.INITIAL_SCHEMA_ID); assertSameSchemaList(ImmutableList.of(schema), freshTable.schemas()); assertThat(freshTable.schema().asStruct()).isEqualTo(schema.asStruct()); @@ -1496,8 +1570,9 @@ schema, new Schema(1, schema2.columns()), new Schema(2, schema3.columns())), assertThat(threeSchemaTable.lastColumnId()).isEqualTo(6); } - @Test - public void testCreateV2MetadataThroughTableProperty() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testCreateMetadataThroughTableProperty(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); TableMetadata meta = @@ -1505,9 +1580,9 @@ public void testCreateV2MetadataThroughTableProperty() { schema, PartitionSpec.unpartitioned(), null, - ImmutableMap.of(TableProperties.FORMAT_VERSION, "2", "key", "val")); + ImmutableMap.of(TableProperties.FORMAT_VERSION, String.valueOf(formatVersion), "key", "val")); - assertThat(meta.formatVersion()).isEqualTo(2); + assertThat(meta.formatVersion()).isEqualTo(formatVersion); assertThat(meta.properties()) .containsEntry("key", "val") .doesNotContainKey(TableProperties.FORMAT_VERSION); @@ -1579,6 +1654,7 @@ public void testUpgradeMetadataThroughTableProperty(int baseFormatVersion, int n .containsExactly(entry("key2", "val2")); } + // TODO: need add v3 @Test public void testParseStatisticsFiles() throws Exception { String data = readTableMetadataInputFile("TableMetadataStatisticsFiles.json"); @@ -1598,6 +1674,7 @@ public void testParseStatisticsFiles() throws Exception { "ndv", 3055729675574597004L, 1, ImmutableList.of(1), ImmutableMap.of())))); } + // TODO: need add v3 @Test public void testParsePartitionStatisticsFiles() throws Exception { String data = readTableMetadataInputFile("TableMetadataPartitionStatisticsFiles.json"); @@ -1613,8 +1690,9 @@ public void testParsePartitionStatisticsFiles() throws Exception { .build()); } - @Test - public void testNoReservedPropertyForTableMetadataCreation() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testNoReservedPropertyForTableMetadataCreation(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); assertThatThrownBy( @@ -1624,11 +1702,11 @@ public void testNoReservedPropertyForTableMetadataCreation() { PartitionSpec.unpartitioned(), null, "/tmp", - ImmutableMap.of(TableProperties.FORMAT_VERSION, "1"), - 1)) + ImmutableMap.of(TableProperties.FORMAT_VERSION, String.valueOf(formatVersion)), + formatVersion)) .isInstanceOf(IllegalArgumentException.class) .hasMessage( - "Table properties should not contain reserved properties, but got {format-version=1}"); + String.format("Table properties should not contain reserved properties, but got {format-version=%s}", formatVersion)); assertThatThrownBy( () -> @@ -1638,18 +1716,19 @@ public void testNoReservedPropertyForTableMetadataCreation() { null, "/tmp", ImmutableMap.of(TableProperties.UUID, "uuid"), - 1)) + formatVersion)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Table properties should not contain reserved properties, but got {uuid=uuid}"); } - @Test - public void testNoTrailingLocationSlash() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testNoTrailingLocationSlash(int formatVersion) { String locationWithSlash = "/with_trailing_slash/"; String locationWithoutSlash = "/with_trailing_slash"; TableMetadata meta = TableMetadata.newTableMetadata( - TEST_SCHEMA, SPEC_5, SORT_ORDER_3, locationWithSlash, Collections.emptyMap()); + TEST_SCHEMA, SPEC_5, SORT_ORDER_3, locationWithSlash, Collections.emptyMap(), formatVersion); assertThat(meta.location()) .as("Metadata should never return a location ending in a slash") .isEqualTo(locationWithoutSlash); @@ -1669,6 +1748,7 @@ private String createManifestListWithManifestFile( return localInput(manifestList).location(); } + // TODO: need v3 @Test public void buildReplacementKeepsSnapshotLog() throws Exception { TableMetadata metadata = @@ -1703,11 +1783,12 @@ public void testConstructV3Metadata() { 3); } - @Test - public void onlyMetadataLocationIsUpdatedWithoutTimestampAndMetadataLogEntry() { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void onlyMetadataLocationIsUpdatedWithoutTimestampAndMetadataLogEntry(int formatVersion) { String uuid = "386b9f01-002b-4d8c-b77f-42c3fd3b7c9b"; TableMetadata metadata = - TableMetadata.buildFromEmpty() + TableMetadata.buildFromEmpty(formatVersion) .assignUUID(uuid) .setLocation("location") .setCurrentSchema(TEST_SCHEMA, 3) diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadataSerialization.java b/core/src/test/java/org/apache/iceberg/TestTableMetadataSerialization.java index 94a3d35b35a6..63925aca1cc7 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadataSerialization.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadataSerialization.java @@ -72,5 +72,7 @@ public void testSerialization() throws Exception { assertThat(Lists.transform(result.snapshots(), Snapshot::snapshotId)) .isEqualTo(Lists.transform(meta.snapshots(), Snapshot::snapshotId)); assertThat(result.snapshotLog()).isEqualTo(meta.snapshotLog()); + assertThat(result.rowLinageEnabled()).isEqualTo(meta.rowLinageEnabled()); + assertThat(result.nextRowId()).isEqualTo(meta.nextRowId()); } } From 92c616b8cd0b08fb8f5ecc8d7aac8b17f7f6daa1 Mon Sep 17 00:00:00 2001 From: Honah J Date: Thu, 5 Dec 2024 15:09:58 -0800 Subject: [PATCH 02/15] remove unnecessary test --- .../java/org/apache/iceberg/TestTableMetadata.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 80f4405bc2cc..9e74817cda5a 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -1772,17 +1772,6 @@ public void buildReplacementKeepsSnapshotLog() throws Exception { .containsExactlyElementsOf(metadata.snapshotLog()); } - @Test - public void testConstructV3Metadata() { - TableMetadata.newTableMetadata( - TEST_SCHEMA, - PartitionSpec.unpartitioned(), - SortOrder.unsorted(), - TEST_LOCATION, - ImmutableMap.of(), - 3); - } - @ParameterizedTest @MethodSource("formatVersionsProvider") public void onlyMetadataLocationIsUpdatedWithoutTimestampAndMetadataLogEntry(int formatVersion) { From 74bc7243e52d7cdcca6c02c234722b1955148122 Mon Sep 17 00:00:00 2001 From: Honah J Date: Sun, 8 Dec 2024 18:16:52 -0800 Subject: [PATCH 03/15] add unit tests for new fields --- .../org/apache/iceberg/TestTableMetadata.java | 247 +++++++++++++++--- 1 file changed, 206 insertions(+), 41 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 9e74817cda5a..22b9ce82aa89 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -97,12 +97,10 @@ public class TestTableMetadata { public TableOperations ops = new LocalTableOperations(temp); - private static Stream formatVersionsProvider() { return Stream.of(1, 2, 3); } - @ParameterizedTest @MethodSource("formatVersionsProvider") @SuppressWarnings("MethodLength") @@ -172,7 +170,7 @@ public void testJsonConversion(int formatVersion) throws Exception { TableMetadata expected = new TableMetadata( null, - formatVersion, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, SEQ_NO, @@ -376,7 +374,7 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { () -> new TableMetadata( null, - formatVersion, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, SEQ_NO, @@ -426,7 +424,7 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { () -> new TableMetadata( null, - formatVersion, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, SEQ_NO, @@ -471,7 +469,7 @@ public void testBranchSnapshotMissing(int formatVersion) { () -> new TableMetadata( null, - formatVersion, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, SEQ_NO, @@ -578,7 +576,7 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception TableMetadata base = new TableMetadata( null, - formatVersion, + formatVersion, UUID.randomUUID().toString(), TEST_LOCATION, 0, @@ -1109,7 +1107,8 @@ public void testNewTableMetadataReassignmentAllIds(int formatVersion) throws Exc .build(); String location = "file://tmp/db/table"; TableMetadata metadata = - TableMetadata.newTableMetadata(schema, spec, SortOrder.unsorted(), location, ImmutableMap.of(), formatVersion); + TableMetadata.newTableMetadata( + schema, spec, SortOrder.unsorted(), location, ImmutableMap.of(), formatVersion); // newTableMetadata should reassign column ids and partition field ids. PartitionSpec expected = @@ -1189,36 +1188,36 @@ public void testBuildReplacementForV2AndV3Table(int formatVersion) { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); Schema schema = - new Schema( - Types.NestedField.required(1, "x", Types.LongType.get()), - Types.NestedField.required(2, "y", Types.LongType.get())); + new Schema( + Types.NestedField.required(1, "x", Types.LongType.get()), + Types.NestedField.required(2, "y", Types.LongType.get())); PartitionSpec spec = - PartitionSpec.builderFor(schema).withSpecId(0).identity("x").identity("y").build(); + PartitionSpec.builderFor(schema).withSpecId(0).identity("x").identity("y").build(); String location = "file://tmp/db/table"; TableMetadata metadata = - TableMetadata.newTableMetadata( - schema, spec, SortOrder.unsorted(), location, ImmutableMap.of(), formatVersion); + TableMetadata.newTableMetadata( + schema, spec, SortOrder.unsorted(), location, ImmutableMap.of(), formatVersion); assertThat(metadata.spec()).isEqualTo(spec); Schema updatedSchema = - new Schema( - Types.NestedField.required(1, "x", Types.LongType.get()), - Types.NestedField.required(2, "z", Types.StringType.get())); + new Schema( + Types.NestedField.required(1, "x", Types.LongType.get()), + Types.NestedField.required(2, "z", Types.StringType.get())); PartitionSpec updatedSpec = - PartitionSpec.builderFor(updatedSchema).withSpecId(0).bucket("z", 8).identity("x").build(); + PartitionSpec.builderFor(updatedSchema).withSpecId(0).bucket("z", 8).identity("x").build(); TableMetadata updated = - metadata.buildReplacement( - updatedSchema, updatedSpec, SortOrder.unsorted(), location, ImmutableMap.of()); + metadata.buildReplacement( + updatedSchema, updatedSpec, SortOrder.unsorted(), location, ImmutableMap.of()); PartitionSpec expected = - PartitionSpec.builderFor(updated.schema()) - .withSpecId(1) - .add(3, 1002, "z_bucket", Transforms.bucket(8)) - .add(1, 1000, "x", Transforms.identity()) - .build(); + PartitionSpec.builderFor(updated.schema()) + .withSpecId(1) + .add(3, 1002, "z_bucket", Transforms.bucket(8)) + .add(1, 1000, "x", Transforms.identity()) + .build(); assertThat(updated.spec()) - .as( - "Should reassign the partition field IDs and reuse any existing IDs for equivalent fields") - .isEqualTo(expected); + .as( + "Should reassign the partition field IDs and reuse any existing IDs for equivalent fields") + .isEqualTo(expected); } @ParameterizedTest @@ -1228,7 +1227,10 @@ public void testSortOrder(int formatVersion) { TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), null, ImmutableMap.of("format-version", String.valueOf(formatVersion))); + schema, + PartitionSpec.unpartitioned(), + null, + ImmutableMap.of("format-version", String.valueOf(formatVersion))); assertThat(meta.formatVersion()).isEqualTo(formatVersion); assertThat(meta.sortOrder().isUnsorted()).isTrue(); assertThat(meta.replaceSortOrder(SortOrder.unsorted())) @@ -1245,7 +1247,11 @@ public void testUpdateSortOrder(int formatVersion) { TableMetadata sortedByX = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), order, null, ImmutableMap.of("format-version", String.valueOf(formatVersion))); + schema, + PartitionSpec.unpartitioned(), + order, + null, + ImmutableMap.of("format-version", String.valueOf(formatVersion))); assertThat(sortedByX.formatVersion()).isEqualTo(formatVersion); assertThat(sortedByX.sortOrders()).hasSize(1); assertThat(sortedByX.sortOrder().orderId()).isEqualTo(1); @@ -1284,7 +1290,12 @@ public void testStatistics(int formatVersion) { TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); + schema, + PartitionSpec.unpartitioned(), + SortOrder.unsorted(), + null, + ImmutableMap.of(), + formatVersion); assertThat(meta.statisticsFiles()).as("Should default to no statistics files").isEmpty(); } @@ -1295,7 +1306,12 @@ public void testSetStatistics(int formatVersion) { TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); + schema, + PartitionSpec.unpartitioned(), + SortOrder.unsorted(), + null, + ImmutableMap.of(), + formatVersion); TableMetadata withStatistics = TableMetadata.buildFrom(meta) @@ -1336,7 +1352,12 @@ public void testRemoveStatistics(int formatVersion) { TableMetadata meta = TableMetadata.buildFrom( TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion)) + schema, + PartitionSpec.unpartitioned(), + SortOrder.unsorted(), + null, + ImmutableMap.of(), + formatVersion)) .setStatistics( 43, new GenericStatisticsFile( @@ -1368,7 +1389,12 @@ public void testPartitionStatistics(int formatVersion) { TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); + schema, + PartitionSpec.unpartitioned(), + SortOrder.unsorted(), + null, + ImmutableMap.of(), + formatVersion); assertThat(meta.partitionStatisticsFiles()) .as("Should default to no partition statistics files") .isEmpty(); @@ -1381,7 +1407,12 @@ public void testSetPartitionStatistics(int formatVersion) { TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); + schema, + PartitionSpec.unpartitioned(), + SortOrder.unsorted(), + null, + ImmutableMap.of(), + formatVersion); TableMetadata withPartitionStatistics = TableMetadata.buildFrom(meta) @@ -1432,7 +1463,12 @@ public void testRemovePartitionStatistics(int formatVersion) { TableMetadata meta = TableMetadata.buildFrom( TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion)) + schema, + PartitionSpec.unpartitioned(), + SortOrder.unsorted(), + null, + ImmutableMap.of(), + formatVersion)) .setPartitionStatistics( ImmutableGenericPartitionStatisticsFile.builder() .snapshotId(43) @@ -1492,7 +1528,12 @@ public void testUpdateSchemaIdentifierFields(int formatVersion) { TableMetadata meta = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); + schema, + PartitionSpec.unpartitioned(), + SortOrder.unsorted(), + null, + ImmutableMap.of(), + formatVersion); Schema newSchema = new Schema( @@ -1510,7 +1551,12 @@ public void testUpdateSchema(int formatVersion) { new Schema(0, Types.NestedField.required(1, "y", Types.LongType.get(), "comment")); TableMetadata freshTable = TableMetadata.newTableMetadata( - schema, PartitionSpec.unpartitioned(), SortOrder.unsorted(), null, ImmutableMap.of(), formatVersion); + schema, + PartitionSpec.unpartitioned(), + SortOrder.unsorted(), + null, + ImmutableMap.of(), + formatVersion); assertThat(freshTable.currentSchemaId()).isEqualTo(TableMetadata.INITIAL_SCHEMA_ID); assertSameSchemaList(ImmutableList.of(schema), freshTable.schemas()); assertThat(freshTable.schema().asStruct()).isEqualTo(schema.asStruct()); @@ -1580,7 +1626,8 @@ public void testCreateMetadataThroughTableProperty(int formatVersion) { schema, PartitionSpec.unpartitioned(), null, - ImmutableMap.of(TableProperties.FORMAT_VERSION, String.valueOf(formatVersion), "key", "val")); + ImmutableMap.of( + TableProperties.FORMAT_VERSION, String.valueOf(formatVersion), "key", "val")); assertThat(meta.formatVersion()).isEqualTo(formatVersion); assertThat(meta.properties()) @@ -1706,7 +1753,9 @@ public void testNoReservedPropertyForTableMetadataCreation(int formatVersion) { formatVersion)) .isInstanceOf(IllegalArgumentException.class) .hasMessage( - String.format("Table properties should not contain reserved properties, but got {format-version=%s}", formatVersion)); + String.format( + "Table properties should not contain reserved properties, but got {format-version=%s}", + formatVersion)); assertThatThrownBy( () -> @@ -1728,7 +1777,12 @@ public void testNoTrailingLocationSlash(int formatVersion) { String locationWithoutSlash = "/with_trailing_slash"; TableMetadata meta = TableMetadata.newTableMetadata( - TEST_SCHEMA, SPEC_5, SORT_ORDER_3, locationWithSlash, Collections.emptyMap(), formatVersion); + TEST_SCHEMA, + SPEC_5, + SORT_ORDER_3, + locationWithSlash, + Collections.emptyMap(), + formatVersion); assertThat(meta.location()) .as("Metadata should never return a location ending in a slash") .isEqualTo(locationWithoutSlash); @@ -1806,4 +1860,115 @@ public void onlyMetadataLocationIsUpdatedWithoutTimestampAndMetadataLogEntry(int assertThat(updatedMetadata.metadataFileLocation()).isEqualTo("updated-metadata-location"); assertThat(updatedMetadata.previousFiles()).isEmpty(); } + + @Test + public void testRowLineageEnabledInV3() { + + TableMetadata meta = + new TableMetadata( + null, + 3, + UUID.randomUUID().toString(), + TEST_LOCATION, + SEQ_NO, + System.currentTimeMillis(), + 3, + 7, + ImmutableList.of(TEST_SCHEMA), + 5, + ImmutableList.of(SPEC_5), + SPEC_5.lastAssignedFieldId(), + 3, + ImmutableList.of(SORT_ORDER_3), + ImmutableMap.of("property", "value"), + -1, + ImmutableList.of(), + null, + ImmutableList.of(), + ImmutableList.of(), + ImmutableMap.of(), + ImmutableList.of(), + ImmutableList.of(), + true, + 0L, + ImmutableList.of()); + + assertThat(meta.rowLinageEnabled()).isTrue(); + assertThat(meta.nextRowId()).isEqualTo(0); + } + + @Test + public void testRowLineageEnabledAndMissingNextRowId() { + assertThatThrownBy( + () -> + new TableMetadata( + null, + 3, + UUID.randomUUID().toString(), + TEST_LOCATION, + SEQ_NO, + System.currentTimeMillis(), + 3, + 7, + ImmutableList.of(TEST_SCHEMA), + 5, + ImmutableList.of(SPEC_5), + SPEC_5.lastAssignedFieldId(), + 3, + ImmutableList.of(SORT_ORDER_3), + ImmutableMap.of("property", "value"), + -1, + ImmutableList.of(), + null, + ImmutableList.of(), + ImmutableList.of(), + ImmutableMap.of(), + ImmutableList.of(), + ImmutableList.of(), + true, + -1L, + ImmutableList.of())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Next row id is required when row lineage is enabled"); + } + + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testRowLineageUnsupportedInV1AndV2(int formatVersion) { + assumeThat(formatVersion).isLessThanOrEqualTo(2); + + assertThatThrownBy( + () -> + new TableMetadata( + null, + formatVersion, + UUID.randomUUID().toString(), + TEST_LOCATION, + 0, + System.currentTimeMillis(), + 3, + 7, + ImmutableList.of(TEST_SCHEMA), + 5, + ImmutableList.of(SPEC_5), + SPEC_5.lastAssignedFieldId(), + 3, + ImmutableList.of(SORT_ORDER_3), + ImmutableMap.of("property", "value"), + -1, + ImmutableList.of(), + null, + ImmutableList.of(), + ImmutableList.of(), + ImmutableMap.of(), + ImmutableList.of(), + ImmutableList.of(), + true, + 0L, + ImmutableList.of())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + String.format( + "Row lineage is only supported in v3 (current version v%s)", formatVersion)); + } } From baf6e0b43f46a51b93e65331dada8fafe6e70135 Mon Sep 17 00:00:00 2001 From: Honah J Date: Sun, 8 Dec 2024 22:35:15 -0800 Subject: [PATCH 04/15] add v3 test metadata json --- .../org/apache/iceberg/TestTableMetadata.java | 130 ++++++++------ ...leMetadataV2PartitionStatisticsFiles.json} | 0 ...on => TableMetadataV2StatisticsFiles.json} | 0 .../TableMetadataV3CurrentSchemaNotFound.json | 146 ++++++++++++++++ ...TableMetadataV3MissingLastPartitionId.json | 145 ++++++++++++++++ .../TableMetadataV3MissingPartitionSpecs.json | 146 ++++++++++++++++ .../TableMetadataV3MissingSchemas.json | 130 ++++++++++++++ .../TableMetadataV3MissingSortOrder.json | 127 ++++++++++++++ ...bleMetadataV3PartitionStatisticsFiles.json | 61 +++++++ .../TableMetadataV3StatisticsFiles.json | 70 ++++++++ .../test/resources/TableMetadataV3Valid.json | 162 ++++++++++++++++++ .../TableMetadataV3ValidMinimal.json | 76 ++++++++ 12 files changed, 1137 insertions(+), 56 deletions(-) rename core/src/test/resources/{TableMetadataPartitionStatisticsFiles.json => TableMetadataV2PartitionStatisticsFiles.json} (100%) rename core/src/test/resources/{TableMetadataStatisticsFiles.json => TableMetadataV2StatisticsFiles.json} (100%) create mode 100644 core/src/test/resources/TableMetadataV3CurrentSchemaNotFound.json create mode 100644 core/src/test/resources/TableMetadataV3MissingLastPartitionId.json create mode 100644 core/src/test/resources/TableMetadataV3MissingPartitionSpecs.json create mode 100644 core/src/test/resources/TableMetadataV3MissingSchemas.json create mode 100644 core/src/test/resources/TableMetadataV3MissingSortOrder.json create mode 100644 core/src/test/resources/TableMetadataV3PartitionStatisticsFiles.json create mode 100644 core/src/test/resources/TableMetadataV3StatisticsFiles.json create mode 100644 core/src/test/resources/TableMetadataV3Valid.json create mode 100644 core/src/test/resources/TableMetadataV3ValidMinimal.json diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 22b9ce82aa89..ddd6d88c9951 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -70,6 +70,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; public class TestTableMetadata { private static final String TEST_LOCATION = "s3://bucket/test/location"; @@ -1021,68 +1022,75 @@ public void testVersionValidation() { .isNotNull(); } - // TODO: add TableMetadataV3Valid.json - @Test - public void testParserVersionValidation() throws Exception { - String supportedVersion1 = readTableMetadataInputFile("TableMetadataV1Valid.json"); - TableMetadata parsed1 = TableMetadataParser.fromJson(supportedVersion1); - assertThat(parsed1).as("Should successfully read supported metadata version").isNotNull(); - - String supportedVersion2 = readTableMetadataInputFile("TableMetadataV2Valid.json"); - TableMetadata parsed2 = TableMetadataParser.fromJson(supportedVersion2); - assertThat(parsed2).as("Should successfully read supported metadata version").isNotNull(); + @ParameterizedTest + @ValueSource(strings = {"TableMetadataV1Valid.json", "TableMetadataV2Valid.json", "TableMetadataV3Valid.json"}) + public void testParserVersionValidation(String fileName) throws Exception { + String supportedVersion = readTableMetadataInputFile(fileName); + TableMetadata parsed = TableMetadataParser.fromJson(supportedVersion); + assertThat(parsed).as("Should successfully read supported metadata version").isNotNull(); + } + @Test + public void testParserUnsupportedVersion() throws Exception { String unsupportedVersion = readTableMetadataInputFile("TableMetadataUnsupportedVersion.json"); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupportedVersion)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Cannot read unsupported version"); + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot read unsupported version"); } - // TODO: add TableMetadataV3Valid.json - @Test - public void testParserV2PartitionSpecsValidation() throws Exception { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testParserV2PartitionSpecsValidation(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + String unsupportedVersion = - readTableMetadataInputFile("TableMetadataV2MissingPartitionSpecs.json"); + readTableMetadataInputFile(String.format("TableMetadataV%sMissingPartitionSpecs.json", formatVersion)); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupportedVersion)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("partition-specs must exist in format v2"); + .hasMessage(String.format("partition-specs must exist in format v%s", formatVersion)); } - // TODO: add TableMetadataV3Valid.json - @Test - public void testParserV2LastAssignedFieldIdValidation() throws Exception { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testParserLastAssignedFieldIdValidation(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + String unsupportedVersion = - readTableMetadataInputFile("TableMetadataV2MissingLastPartitionId.json"); + readTableMetadataInputFile(String.format("TableMetadataV%sMissingLastPartitionId.json", formatVersion)); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupportedVersion)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("last-partition-id must exist in format v2"); + .hasMessage(String.format("last-partition-id must exist in format v%s", formatVersion)); } - // TODO: add TableMetadataV3MissingSortOrder.json - @Test - public void testParserV2SortOrderValidation() throws Exception { - String unsupportedVersion = readTableMetadataInputFile("TableMetadataV2MissingSortOrder.json"); + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testParserSortOrderValidation(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + + String unsupportedVersion = readTableMetadataInputFile(String.format("TableMetadataV%sMissingSortOrder.json", formatVersion)); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupportedVersion)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("sort-orders must exist in format v2"); + .hasMessage(String.format("sort-orders must exist in format v%s", formatVersion)); } - // TODO: add TableMetadataV3CurrentSchemaNotFound.json - @Test - public void testParserV2CurrentSchemaIdValidation() throws Exception { - String unsupported = readTableMetadataInputFile("TableMetadataV2CurrentSchemaNotFound.json"); + @ParameterizedTest + @ValueSource(strings = {"TableMetadataV2CurrentSchemaNotFound.json", "TableMetadataV3CurrentSchemaNotFound.json"}) + public void testParserCurrentSchemaIdValidation(String fileName) throws Exception { + String unsupported = readTableMetadataInputFile(fileName); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupported)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot find schema with current-schema-id=2 from schemas"); } - // TODO: add TableMetadataV3Valid.json - @Test - public void testParserV2SchemasValidation() throws Exception { - String unsupported = readTableMetadataInputFile("TableMetadataV2MissingSchemas.json"); + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testParserV2SchemasValidation(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + + String unsupported = readTableMetadataInputFile(String.format("TableMetadataV%sMissingSchemas.json", formatVersion)); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupported)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("schemas must exist in format v2"); + .hasMessage(String.format("schemas must exist in format v%s", formatVersion)); } private String readTableMetadataInputFile(String fileName) throws Exception { @@ -1501,19 +1509,23 @@ public void testRemovePartitionStatistics(int formatVersion) { assertThat(partitionStatisticsFile.fileSizeInBytes()).isEqualTo(49L); } - // TODO: add V3 - @Test - public void testParseSchemaIdentifierFields() throws Exception { - String data = readTableMetadataInputFile("TableMetadataV2Valid.json"); + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testParseSchemaIdentifierFields(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + + String data = readTableMetadataInputFile(String.format("TableMetadataV%sValid.json", formatVersion)); TableMetadata parsed = TableMetadataParser.fromJson(data); assertThat(parsed.schemasById().get(0).identifierFieldIds()).isEmpty(); assertThat(parsed.schemasById().get(1).identifierFieldIds()).containsExactly(1, 2); } - // TODO: add V3 - @Test - public void testParseMinimal() throws Exception { - String data = readTableMetadataInputFile("TableMetadataV2ValidMinimal.json"); + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testParseMinimal(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + + String data = readTableMetadataInputFile(String.format("TableMetadataV%sValidMinimal.json", formatVersion)); TableMetadata parsed = TableMetadataParser.fromJson(data); assertThat(parsed.snapshots()).isEmpty(); assertThat(parsed.snapshotLog()).isEmpty(); @@ -1701,10 +1713,12 @@ public void testUpgradeMetadataThroughTableProperty(int baseFormatVersion, int n .containsExactly(entry("key2", "val2")); } - // TODO: need add v3 - @Test - public void testParseStatisticsFiles() throws Exception { - String data = readTableMetadataInputFile("TableMetadataStatisticsFiles.json"); + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testParseStatisticsFiles(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + + String data = readTableMetadataInputFile(String.format("TableMetadataV%sStatisticsFiles.json", formatVersion)); TableMetadata parsed = TableMetadataParser.fromJson(data); assertThat(parsed.statisticsFiles()).hasSize(1); assertThat(parsed.statisticsFiles()) @@ -1721,10 +1735,12 @@ public void testParseStatisticsFiles() throws Exception { "ndv", 3055729675574597004L, 1, ImmutableList.of(1), ImmutableMap.of())))); } - // TODO: need add v3 - @Test - public void testParsePartitionStatisticsFiles() throws Exception { - String data = readTableMetadataInputFile("TableMetadataPartitionStatisticsFiles.json"); + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testParsePartitionStatisticsFiles(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + + String data = readTableMetadataInputFile(String.format("TableMetadataV%sPartitionStatisticsFiles.json", formatVersion)); TableMetadata parsed = TableMetadataParser.fromJson(data); assertThat(parsed.partitionStatisticsFiles()) .hasSize(1) @@ -1802,11 +1818,13 @@ private String createManifestListWithManifestFile( return localInput(manifestList).location(); } - // TODO: need v3 - @Test - public void buildReplacementKeepsSnapshotLog() throws Exception { + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void buildReplacementKeepsSnapshotLog(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + TableMetadata metadata = - TableMetadataParser.fromJson(readTableMetadataInputFile("TableMetadataV2Valid.json")); + TableMetadataParser.fromJson(readTableMetadataInputFile(String.format("TableMetadataV%sValid.json", formatVersion))); assertThat(metadata.currentSnapshot()).isNotNull(); assertThat(metadata.snapshots()).hasSize(2); assertThat(metadata.snapshotLog()).hasSize(2); diff --git a/core/src/test/resources/TableMetadataPartitionStatisticsFiles.json b/core/src/test/resources/TableMetadataV2PartitionStatisticsFiles.json similarity index 100% rename from core/src/test/resources/TableMetadataPartitionStatisticsFiles.json rename to core/src/test/resources/TableMetadataV2PartitionStatisticsFiles.json diff --git a/core/src/test/resources/TableMetadataStatisticsFiles.json b/core/src/test/resources/TableMetadataV2StatisticsFiles.json similarity index 100% rename from core/src/test/resources/TableMetadataStatisticsFiles.json rename to core/src/test/resources/TableMetadataV2StatisticsFiles.json diff --git a/core/src/test/resources/TableMetadataV3CurrentSchemaNotFound.json b/core/src/test/resources/TableMetadataV3CurrentSchemaNotFound.json new file mode 100644 index 000000000000..6f1a3d907d7d --- /dev/null +++ b/core/src/test/resources/TableMetadataV3CurrentSchemaNotFound.json @@ -0,0 +1,146 @@ +{ + "format-version": 3, + "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1733710954170, + "last-column-id": 3, + "current-schema-id": 2, + "schemas": [ + { + "type": "struct", + "schema-id": 7, + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + }, + { + "id": 2, + "name": "y", + "required": true, + "type": "long", + "doc": "comment" + }, + { + "id": 3, + "name": "z", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 6, + "fields": [ + { + "id": 10, + "name": "x", + "required": true, + "type": "string" + } + ] + } + ], + "default-spec-id": 5, + "partition-specs": [ + { + "spec-id": 5, + "fields": [] + } + ], + "last-partition-id": 999, + "default-sort-order-id": 3, + "sort-orders": [ + { + "order-id": 3, + "fields": [ + { + "transform": "identity", + "source-id": 2, + "direction": "asc", + "null-order": "nulls-first" + }, + { + "transform": "bucket[4]", + "source-id": 3, + "direction": "desc", + "null-order": "nulls-last" + } + ] + } + ], + "properties": { + "property": "value" + }, + "current-snapshot-id": 1733710954168, + "refs": { + "main": { + "snapshot-id": 1733710954168, + "type": "branch" + }, + "previous": { + "snapshot-id": 1733710953138, + "type": "tag" + }, + "test": { + "snapshot-id": 1733710953138, + "type": "branch" + } + }, + "snapshots": [ + { + "snapshot-id": 1733710953138, + "timestamp-ms": 1733710953138, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17078459248221091859.tmp" + }, + { + "snapshot-id": 1733710954168, + "parent-snapshot-id": 1733710953138, + "timestamp-ms": 1733710954168, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17490643917806818981.tmp", + "schema-id": 7 + } + ], + "statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/stats/file.puffin", + "file-size-in-bytes": 100, + "file-footer-size-in-bytes": 42, + "blob-metadata": [ + { + "type": "some-stats", + "snapshot-id": 11, + "sequence-number": 2, + "fields": [ + 4 + ] + } + ] + } + ], + "partition-statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/partition/stats/file.parquet", + "file-size-in-bytes": 42 + } + ], + "row-lineage": false, + "next-row-id": 0, + "snapshot-log": [ + { + "timestamp-ms": 1733710953138, + "snapshot-id": 1733710953138 + }, + { + "timestamp-ms": 1733710954168, + "snapshot-id": 1733710954168 + } + ], + "metadata-log": [] +} \ No newline at end of file diff --git a/core/src/test/resources/TableMetadataV3MissingLastPartitionId.json b/core/src/test/resources/TableMetadataV3MissingLastPartitionId.json new file mode 100644 index 000000000000..afd819feb8b5 --- /dev/null +++ b/core/src/test/resources/TableMetadataV3MissingLastPartitionId.json @@ -0,0 +1,145 @@ +{ + "format-version": 3, + "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1733710954170, + "last-column-id": 3, + "current-schema-id": 7, + "schemas": [ + { + "type": "struct", + "schema-id": 7, + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + }, + { + "id": 2, + "name": "y", + "required": true, + "type": "long", + "doc": "comment" + }, + { + "id": 3, + "name": "z", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 6, + "fields": [ + { + "id": 10, + "name": "x", + "required": true, + "type": "string" + } + ] + } + ], + "default-spec-id": 5, + "partition-specs": [ + { + "spec-id": 5, + "fields": [] + } + ], + "default-sort-order-id": 3, + "sort-orders": [ + { + "order-id": 3, + "fields": [ + { + "transform": "identity", + "source-id": 2, + "direction": "asc", + "null-order": "nulls-first" + }, + { + "transform": "bucket[4]", + "source-id": 3, + "direction": "desc", + "null-order": "nulls-last" + } + ] + } + ], + "properties": { + "property": "value" + }, + "current-snapshot-id": 1733710954168, + "refs": { + "main": { + "snapshot-id": 1733710954168, + "type": "branch" + }, + "previous": { + "snapshot-id": 1733710953138, + "type": "tag" + }, + "test": { + "snapshot-id": 1733710953138, + "type": "branch" + } + }, + "snapshots": [ + { + "snapshot-id": 1733710953138, + "timestamp-ms": 1733710953138, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17078459248221091859.tmp" + }, + { + "snapshot-id": 1733710954168, + "parent-snapshot-id": 1733710953138, + "timestamp-ms": 1733710954168, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17490643917806818981.tmp", + "schema-id": 7 + } + ], + "statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/stats/file.puffin", + "file-size-in-bytes": 100, + "file-footer-size-in-bytes": 42, + "blob-metadata": [ + { + "type": "some-stats", + "snapshot-id": 11, + "sequence-number": 2, + "fields": [ + 4 + ] + } + ] + } + ], + "partition-statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/partition/stats/file.parquet", + "file-size-in-bytes": 42 + } + ], + "row-lineage": false, + "next-row-id": 0, + "snapshot-log": [ + { + "timestamp-ms": 1733710953138, + "snapshot-id": 1733710953138 + }, + { + "timestamp-ms": 1733710954168, + "snapshot-id": 1733710954168 + } + ], + "metadata-log": [] +} \ No newline at end of file diff --git a/core/src/test/resources/TableMetadataV3MissingPartitionSpecs.json b/core/src/test/resources/TableMetadataV3MissingPartitionSpecs.json new file mode 100644 index 000000000000..ea7e6002856b --- /dev/null +++ b/core/src/test/resources/TableMetadataV3MissingPartitionSpecs.json @@ -0,0 +1,146 @@ +{ + "format-version": 3, + "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1733710954170, + "last-column-id": 3, + "current-schema-id": 7, + "schemas": [ + { + "type": "struct", + "schema-id": 7, + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + }, + { + "id": 2, + "name": "y", + "required": true, + "type": "long", + "doc": "comment" + }, + { + "id": 3, + "name": "z", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 6, + "fields": [ + { + "id": 10, + "name": "x", + "required": true, + "type": "string" + } + ] + } + ], + "partition-spec": [ + { + "name": "x", + "transform": "identity", + "source-id": 1, + "field-id": 1000 + } + ], + "default-sort-order-id": 3, + "sort-orders": [ + { + "order-id": 3, + "fields": [ + { + "transform": "identity", + "source-id": 2, + "direction": "asc", + "null-order": "nulls-first" + }, + { + "transform": "bucket[4]", + "source-id": 3, + "direction": "desc", + "null-order": "nulls-last" + } + ] + } + ], + "properties": { + "property": "value" + }, + "current-snapshot-id": 1733710954168, + "refs": { + "main": { + "snapshot-id": 1733710954168, + "type": "branch" + }, + "previous": { + "snapshot-id": 1733710953138, + "type": "tag" + }, + "test": { + "snapshot-id": 1733710953138, + "type": "branch" + } + }, + "snapshots": [ + { + "snapshot-id": 1733710953138, + "timestamp-ms": 1733710953138, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17078459248221091859.tmp" + }, + { + "snapshot-id": 1733710954168, + "parent-snapshot-id": 1733710953138, + "timestamp-ms": 1733710954168, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17490643917806818981.tmp", + "schema-id": 7 + } + ], + "statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/stats/file.puffin", + "file-size-in-bytes": 100, + "file-footer-size-in-bytes": 42, + "blob-metadata": [ + { + "type": "some-stats", + "snapshot-id": 11, + "sequence-number": 2, + "fields": [ + 4 + ] + } + ] + } + ], + "partition-statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/partition/stats/file.parquet", + "file-size-in-bytes": 42 + } + ], + "row-lineage": false, + "next-row-id": 0, + "snapshot-log": [ + { + "timestamp-ms": 1733710953138, + "snapshot-id": 1733710953138 + }, + { + "timestamp-ms": 1733710954168, + "snapshot-id": 1733710954168 + } + ], + "metadata-log": [] +} \ No newline at end of file diff --git a/core/src/test/resources/TableMetadataV3MissingSchemas.json b/core/src/test/resources/TableMetadataV3MissingSchemas.json new file mode 100644 index 000000000000..a4da9157cf38 --- /dev/null +++ b/core/src/test/resources/TableMetadataV3MissingSchemas.json @@ -0,0 +1,130 @@ +{ + "format-version": 3, + "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1733710954170, + "last-column-id": 3, + "schema": { + "type": "struct", + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + }, + { + "id": 2, + "name": "y", + "required": true, + "type": "long", + "doc": "comment" + }, + { + "id": 3, + "name": "z", + "required": true, + "type": "long" + } + ] + }, + "default-spec-id": 5, + "partition-specs": [ + { + "spec-id": 5, + "fields": [] + } + ], + "last-partition-id": 999, + "default-sort-order-id": 3, + "sort-orders": [ + { + "order-id": 3, + "fields": [ + { + "transform": "identity", + "source-id": 2, + "direction": "asc", + "null-order": "nulls-first" + }, + { + "transform": "bucket[4]", + "source-id": 3, + "direction": "desc", + "null-order": "nulls-last" + } + ] + } + ], + "properties": { + "property": "value" + }, + "current-snapshot-id": 1733710954168, + "refs": { + "main": { + "snapshot-id": 1733710954168, + "type": "branch" + }, + "previous": { + "snapshot-id": 1733710953138, + "type": "tag" + }, + "test": { + "snapshot-id": 1733710953138, + "type": "branch" + } + }, + "snapshots": [ + { + "snapshot-id": 1733710953138, + "timestamp-ms": 1733710953138, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17078459248221091859.tmp" + }, + { + "snapshot-id": 1733710954168, + "parent-snapshot-id": 1733710953138, + "timestamp-ms": 1733710954168, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17490643917806818981.tmp", + "schema-id": 7 + } + ], + "statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/stats/file.puffin", + "file-size-in-bytes": 100, + "file-footer-size-in-bytes": 42, + "blob-metadata": [ + { + "type": "some-stats", + "snapshot-id": 11, + "sequence-number": 2, + "fields": [ + 4 + ] + } + ] + } + ], + "partition-statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/partition/stats/file.parquet", + "file-size-in-bytes": 42 + } + ], + "row-lineage": false, + "next-row-id": 0, + "snapshot-log": [ + { + "timestamp-ms": 1733710953138, + "snapshot-id": 1733710953138 + }, + { + "timestamp-ms": 1733710954168, + "snapshot-id": 1733710954168 + } + ], + "metadata-log": [] +} \ No newline at end of file diff --git a/core/src/test/resources/TableMetadataV3MissingSortOrder.json b/core/src/test/resources/TableMetadataV3MissingSortOrder.json new file mode 100644 index 000000000000..80a3dd11d5f2 --- /dev/null +++ b/core/src/test/resources/TableMetadataV3MissingSortOrder.json @@ -0,0 +1,127 @@ +{ + "format-version": 3, + "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1733710954170, + "last-column-id": 3, + "current-schema-id": 7, + "schemas": [ + { + "type": "struct", + "schema-id": 7, + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + }, + { + "id": 2, + "name": "y", + "required": true, + "type": "long", + "doc": "comment" + }, + { + "id": 3, + "name": "z", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 6, + "fields": [ + { + "id": 10, + "name": "x", + "required": true, + "type": "string" + } + ] + } + ], + "default-spec-id": 5, + "partition-specs": [ + { + "spec-id": 5, + "fields": [] + } + ], + "last-partition-id": 999, + "default-sort-order-id": 3, + "properties": { + "property": "value" + }, + "current-snapshot-id": 1733710954168, + "refs": { + "main": { + "snapshot-id": 1733710954168, + "type": "branch" + }, + "previous": { + "snapshot-id": 1733710953138, + "type": "tag" + }, + "test": { + "snapshot-id": 1733710953138, + "type": "branch" + } + }, + "snapshots": [ + { + "snapshot-id": 1733710953138, + "timestamp-ms": 1733710953138, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17078459248221091859.tmp" + }, + { + "snapshot-id": 1733710954168, + "parent-snapshot-id": 1733710953138, + "timestamp-ms": 1733710954168, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17490643917806818981.tmp", + "schema-id": 7 + } + ], + "statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/stats/file.puffin", + "file-size-in-bytes": 100, + "file-footer-size-in-bytes": 42, + "blob-metadata": [ + { + "type": "some-stats", + "snapshot-id": 11, + "sequence-number": 2, + "fields": [ + 4 + ] + } + ] + } + ], + "partition-statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/partition/stats/file.parquet", + "file-size-in-bytes": 42 + } + ], + "row-lineage": false, + "next-row-id": 0, + "snapshot-log": [ + { + "timestamp-ms": 1733710953138, + "snapshot-id": 1733710953138 + }, + { + "timestamp-ms": 1733710954168, + "snapshot-id": 1733710954168 + } + ], + "metadata-log": [] +} \ No newline at end of file diff --git a/core/src/test/resources/TableMetadataV3PartitionStatisticsFiles.json b/core/src/test/resources/TableMetadataV3PartitionStatisticsFiles.json new file mode 100644 index 000000000000..796f5bc8b13b --- /dev/null +++ b/core/src/test/resources/TableMetadataV3PartitionStatisticsFiles.json @@ -0,0 +1,61 @@ +{ + "format-version": 3, + "table-uuid": "9c12d441-03fe-4693-9a96-a0705ddf69c1", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1602638573590, + "last-column-id": 3, + "current-schema-id": 0, + "schemas": [ + { + "type": "struct", + "schema-id": 0, + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + } + ] + } + ], + "default-spec-id": 0, + "partition-specs": [ + { + "spec-id": 0, + "fields": [] + } + ], + "last-partition-id": 1000, + "default-sort-order-id": 0, + "sort-orders": [ + { + "order-id": 0, + "fields": [] + } + ], + "properties": {}, + "current-snapshot-id": 3055729675574597004, + "snapshots": [ + { + "snapshot-id": 3055729675574597004, + "timestamp-ms": 1555100955770, + "sequence-number": 1, + "summary": { + "operation": "append" + }, + "manifest-list": "s3://a/b/2.avro", + "schema-id": 0 + } + ], + "partition-statistics": [ + { + "snapshot-id": 3055729675574597004, + "statistics-path": "s3://a/b/partition-stats.parquet", + "file-size-in-bytes": 43 + } + ], + "snapshot-log": [], + "metadata-log": [] +} diff --git a/core/src/test/resources/TableMetadataV3StatisticsFiles.json b/core/src/test/resources/TableMetadataV3StatisticsFiles.json new file mode 100644 index 000000000000..6768f83f7a34 --- /dev/null +++ b/core/src/test/resources/TableMetadataV3StatisticsFiles.json @@ -0,0 +1,70 @@ +{ + "format-version": 3, + "table-uuid": "9c12d441-03fe-4693-9a96-a0705ddf69c1", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1602638573590, + "last-column-id": 3, + "current-schema-id": 0, + "schemas": [ + { + "type": "struct", + "schema-id": 0, + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + } + ] + } + ], + "default-spec-id": 0, + "partition-specs": [ + { + "spec-id": 0, + "fields": [] + } + ], + "last-partition-id": 1000, + "default-sort-order-id": 0, + "sort-orders": [ + { + "order-id": 0, + "fields": [] + } + ], + "properties": {}, + "current-snapshot-id": 3055729675574597004, + "snapshots": [ + { + "snapshot-id": 3055729675574597004, + "timestamp-ms": 1555100955770, + "sequence-number": 1, + "summary": { + "operation": "append" + }, + "manifest-list": "s3://a/b/2.avro", + "schema-id": 0 + } + ], + "statistics": [ + { + "snapshot-id": 3055729675574597004, + "statistics-path": "s3://a/b/stats.puffin", + "file-size-in-bytes": 413, + "file-footer-size-in-bytes": 42, + "blob-metadata": [ + { + "type": "ndv", + "snapshot-id": 3055729675574597004, + "sequence-number": 1, + "fields": [1] + } + ] + } + ], + "snapshot-log": [], + "metadata-log": [] +} \ No newline at end of file diff --git a/core/src/test/resources/TableMetadataV3Valid.json b/core/src/test/resources/TableMetadataV3Valid.json new file mode 100644 index 000000000000..a65b77b4c0d2 --- /dev/null +++ b/core/src/test/resources/TableMetadataV3Valid.json @@ -0,0 +1,162 @@ +{ + "format-version": 3, + "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1733710954170, + "last-column-id": 3, + "current-schema-id": 1, + "schemas": [ + { + "type": "struct", + "schema-id": 0, + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 1, + "identifier-field-ids": [ + 1, + 2 + ], + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + }, + { + "id": 2, + "name": "y", + "required": true, + "type": "long", + "doc": "comment" + }, + { + "id": 3, + "name": "z", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 6, + "fields": [ + { + "id": 10, + "name": "x", + "required": true, + "type": "string" + } + ] + } + ], + "default-spec-id": 5, + "partition-specs": [ + { + "spec-id": 5, + "fields": [] + } + ], + "last-partition-id": 999, + "default-sort-order-id": 3, + "sort-orders": [ + { + "order-id": 3, + "fields": [ + { + "transform": "identity", + "source-id": 2, + "direction": "asc", + "null-order": "nulls-first" + }, + { + "transform": "bucket[4]", + "source-id": 3, + "direction": "desc", + "null-order": "nulls-last" + } + ] + } + ], + "properties": { + "property": "value" + }, + "current-snapshot-id": 1733710954168, + "refs": { + "main": { + "snapshot-id": 1733710954168, + "type": "branch" + }, + "previous": { + "snapshot-id": 1733710953138, + "type": "tag" + }, + "test": { + "snapshot-id": 1733710953138, + "type": "branch" + } + }, + "snapshots": [ + { + "snapshot-id": 1733710953138, + "timestamp-ms": 1733710953138, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17078459248221091859.tmp" + }, + { + "snapshot-id": 1733710954168, + "parent-snapshot-id": 1733710953138, + "timestamp-ms": 1733710954168, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17490643917806818981.tmp", + "schema-id": 7 + } + ], + "statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/stats/file.puffin", + "file-size-in-bytes": 100, + "file-footer-size-in-bytes": 42, + "blob-metadata": [ + { + "type": "some-stats", + "snapshot-id": 11, + "sequence-number": 2, + "fields": [ + 4 + ] + } + ] + } + ], + "partition-statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/partition/stats/file.parquet", + "file-size-in-bytes": 42 + } + ], + "row-lineage": false, + "next-row-id": 0, + "snapshot-log": [ + { + "timestamp-ms": 1733710953138, + "snapshot-id": 1733710953138 + }, + { + "timestamp-ms": 1733710954168, + "snapshot-id": 1733710954168 + } + ], + "metadata-log": [] +} \ No newline at end of file diff --git a/core/src/test/resources/TableMetadataV3ValidMinimal.json b/core/src/test/resources/TableMetadataV3ValidMinimal.json new file mode 100644 index 000000000000..a6bfb5c7f10f --- /dev/null +++ b/core/src/test/resources/TableMetadataV3ValidMinimal.json @@ -0,0 +1,76 @@ +{ + "format-version": 3, + "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1733710954170, + "last-column-id": 3, + "current-schema-id": 7, + "schemas": [ + { + "type": "struct", + "schema-id": 7, + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + }, + { + "id": 2, + "name": "y", + "required": true, + "type": "long", + "doc": "comment" + }, + { + "id": 3, + "name": "z", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 6, + "fields": [ + { + "id": 10, + "name": "x", + "required": true, + "type": "string" + } + ] + } + ], + "default-spec-id": 5, + "partition-specs": [ + { + "spec-id": 5, + "fields": [] + } + ], + "last-partition-id": 999, + "default-sort-order-id": 3, + "sort-orders": [ + { + "order-id": 3, + "fields": [ + { + "transform": "identity", + "source-id": 2, + "direction": "asc", + "null-order": "nulls-first" + }, + { + "transform": "bucket[4]", + "source-id": 3, + "direction": "desc", + "null-order": "nulls-last" + } + ] + } + ] +} \ No newline at end of file From 4786fbe5c236f1ead5bac6b62631915bef50ba21 Mon Sep 17 00:00:00 2001 From: Honah J Date: Sun, 8 Dec 2024 22:50:35 -0800 Subject: [PATCH 05/15] add tests for v3 row lineage parsing --- .../org/apache/iceberg/TableMetadata.java | 1 - .../apache/iceberg/TableMetadataParser.java | 1 - .../org/apache/iceberg/TestTableMetadata.java | 81 +++++++-- .../TableMetadataV3MissingNextRowId.json | 161 +++++++++++++++++ .../TableMetadataV3RowLineageEnabled.json | 162 ++++++++++++++++++ 5 files changed, 386 insertions(+), 20 deletions(-) create mode 100644 core/src/test/resources/TableMetadataV3MissingNextRowId.json create mode 100644 core/src/test/resources/TableMetadataV3RowLineageEnabled.json diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java index 9f0488366ebc..45ce9e5bf016 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadata.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java @@ -263,7 +263,6 @@ public String toString() { private volatile Map refs; private volatile boolean snapshotsLoaded; private final boolean rowLineageEnabled; - // TODO: may need to use boxing to allow null value private final long nextRowId; @SuppressWarnings("checkstyle:CyclomaticComplexity") diff --git a/core/src/main/java/org/apache/iceberg/TableMetadataParser.java b/core/src/main/java/org/apache/iceberg/TableMetadataParser.java index de15798c54c1..48e399a9ef92 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadataParser.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadataParser.java @@ -533,7 +533,6 @@ public static TableMetadata fromJson(String metadataLocation, JsonNode node) { } boolean rowLineageEnabled = false; - // TODO: do we want to be more strict when parsing? e.g. raise if see row lineage in v2 if (formatVersion == 3 && node.hasNonNull(ROW_LINEAGE)) { rowLineageEnabled = JsonUtil.getBool(ROW_LINEAGE, node); } diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index ddd6d88c9951..1237bb88f58a 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -70,7 +70,6 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import org.junit.jupiter.params.provider.ValueSource; public class TestTableMetadata { private static final String TEST_LOCATION = "s3://bucket/test/location"; @@ -1023,9 +1022,10 @@ public void testVersionValidation() { } @ParameterizedTest - @ValueSource(strings = {"TableMetadataV1Valid.json", "TableMetadataV2Valid.json", "TableMetadataV3Valid.json"}) - public void testParserVersionValidation(String fileName) throws Exception { - String supportedVersion = readTableMetadataInputFile(fileName); + @MethodSource("formatVersionsProvider") + public void testParserVersionValidation(int formatVersion) throws Exception { + String supportedVersion = + readTableMetadataInputFile(String.format("TableMetadataV%sValid.json", formatVersion)); TableMetadata parsed = TableMetadataParser.fromJson(supportedVersion); assertThat(parsed).as("Should successfully read supported metadata version").isNotNull(); } @@ -1034,8 +1034,8 @@ public void testParserVersionValidation(String fileName) throws Exception { public void testParserUnsupportedVersion() throws Exception { String unsupportedVersion = readTableMetadataInputFile("TableMetadataUnsupportedVersion.json"); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupportedVersion)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageStartingWith("Cannot read unsupported version"); + .isInstanceOf(IllegalArgumentException.class) + .hasMessageStartingWith("Cannot read unsupported version"); } @ParameterizedTest @@ -1044,7 +1044,8 @@ public void testParserV2PartitionSpecsValidation(int formatVersion) throws Excep assumeThat(formatVersion).isGreaterThanOrEqualTo(2); String unsupportedVersion = - readTableMetadataInputFile(String.format("TableMetadataV%sMissingPartitionSpecs.json", formatVersion)); + readTableMetadataInputFile( + String.format("TableMetadataV%sMissingPartitionSpecs.json", formatVersion)); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupportedVersion)) .isInstanceOf(IllegalArgumentException.class) .hasMessage(String.format("partition-specs must exist in format v%s", formatVersion)); @@ -1056,7 +1057,8 @@ public void testParserLastAssignedFieldIdValidation(int formatVersion) throws Ex assumeThat(formatVersion).isGreaterThanOrEqualTo(2); String unsupportedVersion = - readTableMetadataInputFile(String.format("TableMetadataV%sMissingLastPartitionId.json", formatVersion)); + readTableMetadataInputFile( + String.format("TableMetadataV%sMissingLastPartitionId.json", formatVersion)); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupportedVersion)) .isInstanceOf(IllegalArgumentException.class) .hasMessage(String.format("last-partition-id must exist in format v%s", formatVersion)); @@ -1067,16 +1069,22 @@ public void testParserLastAssignedFieldIdValidation(int formatVersion) throws Ex public void testParserSortOrderValidation(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); - String unsupportedVersion = readTableMetadataInputFile(String.format("TableMetadataV%sMissingSortOrder.json", formatVersion)); + String unsupportedVersion = + readTableMetadataInputFile( + String.format("TableMetadataV%sMissingSortOrder.json", formatVersion)); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupportedVersion)) .isInstanceOf(IllegalArgumentException.class) .hasMessage(String.format("sort-orders must exist in format v%s", formatVersion)); } @ParameterizedTest - @ValueSource(strings = {"TableMetadataV2CurrentSchemaNotFound.json", "TableMetadataV3CurrentSchemaNotFound.json"}) - public void testParserCurrentSchemaIdValidation(String fileName) throws Exception { - String unsupported = readTableMetadataInputFile(fileName); + @MethodSource("formatVersionsProvider") + public void testParserCurrentSchemaIdValidation(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + + String unsupported = + readTableMetadataInputFile( + String.format("TableMetadataV%sCurrentSchemaNotFound.json", formatVersion)); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupported)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Cannot find schema with current-schema-id=2 from schemas"); @@ -1087,7 +1095,9 @@ public void testParserCurrentSchemaIdValidation(String fileName) throws Exceptio public void testParserV2SchemasValidation(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); - String unsupported = readTableMetadataInputFile(String.format("TableMetadataV%sMissingSchemas.json", formatVersion)); + String unsupported = + readTableMetadataInputFile( + String.format("TableMetadataV%sMissingSchemas.json", formatVersion)); assertThatThrownBy(() -> TableMetadataParser.fromJson(unsupported)) .isInstanceOf(IllegalArgumentException.class) .hasMessage(String.format("schemas must exist in format v%s", formatVersion)); @@ -1514,7 +1524,8 @@ public void testRemovePartitionStatistics(int formatVersion) { public void testParseSchemaIdentifierFields(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); - String data = readTableMetadataInputFile(String.format("TableMetadataV%sValid.json", formatVersion)); + String data = + readTableMetadataInputFile(String.format("TableMetadataV%sValid.json", formatVersion)); TableMetadata parsed = TableMetadataParser.fromJson(data); assertThat(parsed.schemasById().get(0).identifierFieldIds()).isEmpty(); assertThat(parsed.schemasById().get(1).identifierFieldIds()).containsExactly(1, 2); @@ -1525,7 +1536,9 @@ public void testParseSchemaIdentifierFields(int formatVersion) throws Exception public void testParseMinimal(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); - String data = readTableMetadataInputFile(String.format("TableMetadataV%sValidMinimal.json", formatVersion)); + String data = + readTableMetadataInputFile( + String.format("TableMetadataV%sValidMinimal.json", formatVersion)); TableMetadata parsed = TableMetadataParser.fromJson(data); assertThat(parsed.snapshots()).isEmpty(); assertThat(parsed.snapshotLog()).isEmpty(); @@ -1718,7 +1731,9 @@ public void testUpgradeMetadataThroughTableProperty(int baseFormatVersion, int n public void testParseStatisticsFiles(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); - String data = readTableMetadataInputFile(String.format("TableMetadataV%sStatisticsFiles.json", formatVersion)); + String data = + readTableMetadataInputFile( + String.format("TableMetadataV%sStatisticsFiles.json", formatVersion)); TableMetadata parsed = TableMetadataParser.fromJson(data); assertThat(parsed.statisticsFiles()).hasSize(1); assertThat(parsed.statisticsFiles()) @@ -1740,7 +1755,9 @@ public void testParseStatisticsFiles(int formatVersion) throws Exception { public void testParsePartitionStatisticsFiles(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); - String data = readTableMetadataInputFile(String.format("TableMetadataV%sPartitionStatisticsFiles.json", formatVersion)); + String data = + readTableMetadataInputFile( + String.format("TableMetadataV%sPartitionStatisticsFiles.json", formatVersion)); TableMetadata parsed = TableMetadataParser.fromJson(data); assertThat(parsed.partitionStatisticsFiles()) .hasSize(1) @@ -1824,7 +1841,8 @@ public void buildReplacementKeepsSnapshotLog(int formatVersion) throws Exception assumeThat(formatVersion).isGreaterThanOrEqualTo(2); TableMetadata metadata = - TableMetadataParser.fromJson(readTableMetadataInputFile(String.format("TableMetadataV%sValid.json", formatVersion))); + TableMetadataParser.fromJson( + readTableMetadataInputFile(String.format("TableMetadataV%sValid.json", formatVersion))); assertThat(metadata.currentSnapshot()).isNotNull(); assertThat(metadata.snapshots()).hasSize(2); assertThat(metadata.snapshotLog()).hasSize(2); @@ -1989,4 +2007,31 @@ public void testRowLineageUnsupportedInV1AndV2(int formatVersion) { String.format( "Row lineage is only supported in v3 (current version v%s)", formatVersion)); } + + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testParseV3RowLineageEnabled(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(3); + + String data = + readTableMetadataInputFile( + String.format("TableMetadataV%sRowLineageEnabled.json", formatVersion)); + TableMetadata parsed = TableMetadataParser.fromJson(data); + + assertThat(parsed.rowLinageEnabled()).isTrue(); + assertThat(parsed.nextRowId()).isEqualTo(0); + } + + @ParameterizedTest + @MethodSource("formatVersionsProvider") + public void testParseV3RowLineageEnabledAndMissingNextRowId(int formatVersion) throws Exception { + assumeThat(formatVersion).isGreaterThanOrEqualTo(3); + + String data = + readTableMetadataInputFile( + String.format("TableMetadataV%sMissingNextRowId.json", formatVersion)); + assertThatThrownBy(() -> TableMetadataParser.fromJson(data)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Next row must be set when row lineage is enabled"); + } } diff --git a/core/src/test/resources/TableMetadataV3MissingNextRowId.json b/core/src/test/resources/TableMetadataV3MissingNextRowId.json new file mode 100644 index 000000000000..a7e0ea96691d --- /dev/null +++ b/core/src/test/resources/TableMetadataV3MissingNextRowId.json @@ -0,0 +1,161 @@ +{ + "format-version": 3, + "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1733710954170, + "last-column-id": 3, + "current-schema-id": 1, + "schemas": [ + { + "type": "struct", + "schema-id": 0, + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 1, + "identifier-field-ids": [ + 1, + 2 + ], + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + }, + { + "id": 2, + "name": "y", + "required": true, + "type": "long", + "doc": "comment" + }, + { + "id": 3, + "name": "z", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 6, + "fields": [ + { + "id": 10, + "name": "x", + "required": true, + "type": "string" + } + ] + } + ], + "default-spec-id": 5, + "partition-specs": [ + { + "spec-id": 5, + "fields": [] + } + ], + "last-partition-id": 999, + "default-sort-order-id": 3, + "sort-orders": [ + { + "order-id": 3, + "fields": [ + { + "transform": "identity", + "source-id": 2, + "direction": "asc", + "null-order": "nulls-first" + }, + { + "transform": "bucket[4]", + "source-id": 3, + "direction": "desc", + "null-order": "nulls-last" + } + ] + } + ], + "properties": { + "property": "value" + }, + "current-snapshot-id": 1733710954168, + "refs": { + "main": { + "snapshot-id": 1733710954168, + "type": "branch" + }, + "previous": { + "snapshot-id": 1733710953138, + "type": "tag" + }, + "test": { + "snapshot-id": 1733710953138, + "type": "branch" + } + }, + "snapshots": [ + { + "snapshot-id": 1733710953138, + "timestamp-ms": 1733710953138, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17078459248221091859.tmp" + }, + { + "snapshot-id": 1733710954168, + "parent-snapshot-id": 1733710953138, + "timestamp-ms": 1733710954168, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17490643917806818981.tmp", + "schema-id": 7 + } + ], + "statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/stats/file.puffin", + "file-size-in-bytes": 100, + "file-footer-size-in-bytes": 42, + "blob-metadata": [ + { + "type": "some-stats", + "snapshot-id": 11, + "sequence-number": 2, + "fields": [ + 4 + ] + } + ] + } + ], + "partition-statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/partition/stats/file.parquet", + "file-size-in-bytes": 42 + } + ], + "row-lineage": true, + "snapshot-log": [ + { + "timestamp-ms": 1733710953138, + "snapshot-id": 1733710953138 + }, + { + "timestamp-ms": 1733710954168, + "snapshot-id": 1733710954168 + } + ], + "metadata-log": [] +} \ No newline at end of file diff --git a/core/src/test/resources/TableMetadataV3RowLineageEnabled.json b/core/src/test/resources/TableMetadataV3RowLineageEnabled.json new file mode 100644 index 000000000000..75ee92cca6a7 --- /dev/null +++ b/core/src/test/resources/TableMetadataV3RowLineageEnabled.json @@ -0,0 +1,162 @@ +{ + "format-version": 3, + "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", + "location": "s3://bucket/test/location", + "last-sequence-number": 34, + "last-updated-ms": 1733710954170, + "last-column-id": 3, + "current-schema-id": 1, + "schemas": [ + { + "type": "struct", + "schema-id": 0, + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 1, + "identifier-field-ids": [ + 1, + 2 + ], + "fields": [ + { + "id": 1, + "name": "x", + "required": true, + "type": "long" + }, + { + "id": 2, + "name": "y", + "required": true, + "type": "long", + "doc": "comment" + }, + { + "id": 3, + "name": "z", + "required": true, + "type": "long" + } + ] + }, + { + "type": "struct", + "schema-id": 6, + "fields": [ + { + "id": 10, + "name": "x", + "required": true, + "type": "string" + } + ] + } + ], + "default-spec-id": 5, + "partition-specs": [ + { + "spec-id": 5, + "fields": [] + } + ], + "last-partition-id": 999, + "default-sort-order-id": 3, + "sort-orders": [ + { + "order-id": 3, + "fields": [ + { + "transform": "identity", + "source-id": 2, + "direction": "asc", + "null-order": "nulls-first" + }, + { + "transform": "bucket[4]", + "source-id": 3, + "direction": "desc", + "null-order": "nulls-last" + } + ] + } + ], + "properties": { + "property": "value" + }, + "current-snapshot-id": 1733710954168, + "refs": { + "main": { + "snapshot-id": 1733710954168, + "type": "branch" + }, + "previous": { + "snapshot-id": 1733710953138, + "type": "tag" + }, + "test": { + "snapshot-id": 1733710953138, + "type": "branch" + } + }, + "snapshots": [ + { + "snapshot-id": 1733710953138, + "timestamp-ms": 1733710953138, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17078459248221091859.tmp" + }, + { + "snapshot-id": 1733710954168, + "parent-snapshot-id": 1733710953138, + "timestamp-ms": 1733710954168, + "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17490643917806818981.tmp", + "schema-id": 7 + } + ], + "statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/stats/file.puffin", + "file-size-in-bytes": 100, + "file-footer-size-in-bytes": 42, + "blob-metadata": [ + { + "type": "some-stats", + "snapshot-id": 11, + "sequence-number": 2, + "fields": [ + 4 + ] + } + ] + } + ], + "partition-statistics": [ + { + "snapshot-id": 11, + "statistics-path": "/some/partition/stats/file.parquet", + "file-size-in-bytes": 42 + } + ], + "row-lineage": true, + "next-row-id": 0, + "snapshot-log": [ + { + "timestamp-ms": 1733710953138, + "snapshot-id": 1733710953138 + }, + { + "timestamp-ms": 1733710954168, + "snapshot-id": 1733710954168 + } + ], + "metadata-log": [] +} \ No newline at end of file From 4f503704ec88621a90d6b25ef7ef0a3fd146f98d Mon Sep 17 00:00:00 2001 From: Honah J Date: Fri, 10 Jan 2025 11:01:06 -0800 Subject: [PATCH 06/15] Fix Compilation error --- core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java b/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java index f250d2e12289..a714ba3e70a9 100644 --- a/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java +++ b/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java @@ -129,6 +129,8 @@ public static TableMetadata replacePaths( // TODO: update statistic file paths metadata.statisticsFiles(), metadata.partitionStatisticsFiles(), + metadata.rowLinageEnabled(), + metadata.nextRowId(), metadata.changes()); } From 2551587bf9d340507c2a4ca8ee355ee43c02383c Mon Sep 17 00:00:00 2001 From: Honah J Date: Fri, 10 Jan 2025 15:00:11 -0800 Subject: [PATCH 07/15] remove fields for row lineage --- .../apache/iceberg/RewriteTablePathUtil.java | 2 - .../org/apache/iceberg/TableMetadata.java | 29 ---- .../apache/iceberg/TableMetadataParser.java | 21 --- .../org/apache/iceberg/TestTableMetadata.java | 162 ------------------ .../TestTableMetadataSerialization.java | 2 - .../TableMetadataV3MissingNextRowId.json | 161 ----------------- .../TableMetadataV3RowLineageEnabled.json | 162 ------------------ 7 files changed, 539 deletions(-) delete mode 100644 core/src/test/resources/TableMetadataV3MissingNextRowId.json delete mode 100644 core/src/test/resources/TableMetadataV3RowLineageEnabled.json diff --git a/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java b/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java index a714ba3e70a9..f250d2e12289 100644 --- a/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java +++ b/core/src/main/java/org/apache/iceberg/RewriteTablePathUtil.java @@ -129,8 +129,6 @@ public static TableMetadata replacePaths( // TODO: update statistic file paths metadata.statisticsFiles(), metadata.partitionStatisticsFiles(), - metadata.rowLinageEnabled(), - metadata.nextRowId(), metadata.changes()); } diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java index 6a4b527011fd..4ba3bdf8d737 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadata.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java @@ -262,8 +262,6 @@ public String toString() { private volatile Map snapshotsById; private volatile Map refs; private volatile boolean snapshotsLoaded; - private final boolean rowLineageEnabled; - private final long nextRowId; @SuppressWarnings("checkstyle:CyclomaticComplexity") TableMetadata( @@ -290,8 +288,6 @@ public String toString() { Map refs, List statisticsFiles, List partitionStatisticsFiles, - boolean rowLineageEnabled, - long nextRowId, List changes) { Preconditions.checkArgument( specs != null && !specs.isEmpty(), "Partition specs cannot be null or empty"); @@ -311,13 +307,6 @@ public String toString() { Preconditions.checkArgument( metadataFileLocation == null || changes.isEmpty(), "Cannot create TableMetadata with a metadata location and changes"); - Preconditions.checkArgument( - !rowLineageEnabled || formatVersion == 3, - "Row lineage is only supported in v3 (current version v%s)", - formatVersion); - Preconditions.checkArgument( - !rowLineageEnabled || nextRowId >= 0, - "Next row id is required when row lineage is enabled"); this.metadataFileLocation = metadataFileLocation; this.formatVersion = formatVersion; @@ -351,8 +340,6 @@ public String toString() { this.refs = validateRefs(currentSnapshotId, refs, snapshotsById); this.statisticsFiles = ImmutableList.copyOf(statisticsFiles); this.partitionStatisticsFiles = ImmutableList.copyOf(partitionStatisticsFiles); - this.rowLineageEnabled = rowLineageEnabled; - this.nextRowId = nextRowId; HistoryEntry last = null; for (HistoryEntry logEntry : snapshotLog) { @@ -568,14 +555,6 @@ public List previousFiles() { return previousFiles; } - public boolean rowLinageEnabled() { - return rowLineageEnabled; - } - - public long nextRowId() { - return nextRowId; - } - public List changes() { return changes; } @@ -924,8 +903,6 @@ public static class Builder { private final Map> statisticsFiles; private final Map> partitionStatisticsFiles; private boolean suppressHistoricalSnapshots = false; - private boolean rowLinageEnabled; - private long nextRowId; // change tracking private final List changes; @@ -972,8 +949,6 @@ private Builder(int formatVersion) { this.schemasById = Maps.newHashMap(); this.specsById = Maps.newHashMap(); this.sortOrdersById = Maps.newHashMap(); - this.rowLinageEnabled = false; - this.nextRowId = -1L; } private Builder(TableMetadata base) { @@ -996,8 +971,6 @@ private Builder(TableMetadata base) { this.snapshots = Lists.newArrayList(base.snapshots()); this.changes = Lists.newArrayList(base.changes); this.startingChangeCount = changes.size(); - this.rowLinageEnabled = base.rowLineageEnabled; - this.nextRowId = base.nextRowId; this.snapshotLog = Lists.newArrayList(base.snapshotLog); this.previousFileLocation = base.metadataFileLocation; @@ -1562,8 +1535,6 @@ public TableMetadata build() { partitionStatisticsFiles.values().stream() .flatMap(List::stream) .collect(Collectors.toList()), - rowLinageEnabled, - nextRowId, discardChanges ? ImmutableList.of() : ImmutableList.copyOf(changes)); } diff --git a/core/src/main/java/org/apache/iceberg/TableMetadataParser.java b/core/src/main/java/org/apache/iceberg/TableMetadataParser.java index 48e399a9ef92..13633a17f916 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadataParser.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadataParser.java @@ -242,13 +242,6 @@ public static void toJson(TableMetadata metadata, JsonGenerator generator) throw } generator.writeEndArray(); - if (metadata.formatVersion() == 3) { - generator.writeBooleanField(ROW_LINEAGE, metadata.rowLinageEnabled()); - if (metadata.nextRowId() > -1L) { - generator.writeNumberField(NEXT_ROW_ID, metadata.nextRowId()); - } - } - generator.writeArrayFieldStart(SNAPSHOT_LOG); for (HistoryEntry logEntry : metadata.snapshotLog()) { generator.writeStartObject(); @@ -532,18 +525,6 @@ public static TableMetadata fromJson(String metadataLocation, JsonNode node) { } } - boolean rowLineageEnabled = false; - if (formatVersion == 3 && node.hasNonNull(ROW_LINEAGE)) { - rowLineageEnabled = JsonUtil.getBool(ROW_LINEAGE, node); - } - Long nextRowId = JsonUtil.getLongOrNull(NEXT_ROW_ID, node); - Preconditions.checkArgument( - !rowLineageEnabled || nextRowId != null, - "Next row must be set when row lineage is enabled"); - if (nextRowId == null) { - nextRowId = -1L; - } - return new TableMetadata( metadataLocation, formatVersion, @@ -568,8 +549,6 @@ public static TableMetadata fromJson(String metadataLocation, JsonNode node) { refs, statisticsFiles, partitionStatisticsFiles, - rowLineageEnabled, - nextRowId, ImmutableList.of() /* no changes from the file */); } diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 1bd72381e911..a593f3537a01 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -192,8 +192,6 @@ public void testJsonConversion(int formatVersion) throws Exception { refs, statisticsFiles, partitionStatisticsFiles, - false, - 0L, ImmutableList.of()); String asJson = TableMetadataParser.toJson(expected); @@ -283,8 +281,6 @@ public void testBackwardCompat() throws Exception { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of()); String asJson = toJsonWithoutSpecAndSchemaList(expected); @@ -396,8 +392,6 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { refs, ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Current snapshot ID does not match main branch"); @@ -446,8 +440,6 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { refs, ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Current snapshot is not set, but main branch exists"); @@ -491,8 +483,6 @@ public void testBranchSnapshotMissing(int formatVersion) { refs, ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessageEndingWith("does not exist in the existing snapshots list"); @@ -598,8 +588,6 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of()); String asJson = TableMetadataParser.toJson(base); @@ -677,8 +665,6 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of()); previousMetadataLog.add(latestPreviousMetadata); @@ -771,8 +757,6 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of()); previousMetadataLog.add(latestPreviousMetadata); @@ -869,8 +853,6 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of()); previousMetadataLog.add(latestPreviousMetadata); @@ -920,8 +902,6 @@ public void testV2UUIDValidation(int formatVersion) { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessage(String.format("UUID is required in format v%s", formatVersion)); @@ -957,8 +937,6 @@ public void testVersionValidation() { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of())) .isInstanceOf(IllegalArgumentException.class) .hasMessage( @@ -1005,8 +983,6 @@ public void testVersionValidation() { ImmutableMap.of(), ImmutableList.of(), ImmutableList.of(), - false, - -1L, ImmutableList.of())) .isNotNull(); @@ -1915,142 +1891,4 @@ public void onlyMetadataLocationIsUpdatedWithoutTimestampAndMetadataLogEntry(int assertThat(updatedMetadata.metadataFileLocation()).isEqualTo("updated-metadata-location"); assertThat(updatedMetadata.previousFiles()).isEmpty(); } - - @Test - public void testRowLineageEnabledInV3() { - - TableMetadata meta = - new TableMetadata( - null, - 3, - UUID.randomUUID().toString(), - TEST_LOCATION, - SEQ_NO, - System.currentTimeMillis(), - 3, - 7, - ImmutableList.of(TEST_SCHEMA), - 5, - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of("property", "value"), - -1, - ImmutableList.of(), - null, - ImmutableList.of(), - ImmutableList.of(), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - true, - 0L, - ImmutableList.of()); - - assertThat(meta.rowLinageEnabled()).isTrue(); - assertThat(meta.nextRowId()).isEqualTo(0); - } - - @Test - public void testRowLineageEnabledAndMissingNextRowId() { - assertThatThrownBy( - () -> - new TableMetadata( - null, - 3, - UUID.randomUUID().toString(), - TEST_LOCATION, - SEQ_NO, - System.currentTimeMillis(), - 3, - 7, - ImmutableList.of(TEST_SCHEMA), - 5, - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of("property", "value"), - -1, - ImmutableList.of(), - null, - ImmutableList.of(), - ImmutableList.of(), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - true, - -1L, - ImmutableList.of())) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Next row id is required when row lineage is enabled"); - } - - @ParameterizedTest - @MethodSource("formatVersionsProvider") - public void testRowLineageUnsupportedInV1AndV2(int formatVersion) { - assumeThat(formatVersion).isLessThanOrEqualTo(2); - - assertThatThrownBy( - () -> - new TableMetadata( - null, - formatVersion, - UUID.randomUUID().toString(), - TEST_LOCATION, - 0, - System.currentTimeMillis(), - 3, - 7, - ImmutableList.of(TEST_SCHEMA), - 5, - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of("property", "value"), - -1, - ImmutableList.of(), - null, - ImmutableList.of(), - ImmutableList.of(), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - true, - 0L, - ImmutableList.of())) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage( - String.format( - "Row lineage is only supported in v3 (current version v%s)", formatVersion)); - } - - @ParameterizedTest - @MethodSource("formatVersionsProvider") - public void testParseV3RowLineageEnabled(int formatVersion) throws Exception { - assumeThat(formatVersion).isGreaterThanOrEqualTo(3); - - String data = - readTableMetadataInputFile( - String.format("TableMetadataV%sRowLineageEnabled.json", formatVersion)); - TableMetadata parsed = TableMetadataParser.fromJson(data); - - assertThat(parsed.rowLinageEnabled()).isTrue(); - assertThat(parsed.nextRowId()).isEqualTo(0); - } - - @ParameterizedTest - @MethodSource("formatVersionsProvider") - public void testParseV3RowLineageEnabledAndMissingNextRowId(int formatVersion) throws Exception { - assumeThat(formatVersion).isGreaterThanOrEqualTo(3); - - String data = - readTableMetadataInputFile( - String.format("TableMetadataV%sMissingNextRowId.json", formatVersion)); - assertThatThrownBy(() -> TableMetadataParser.fromJson(data)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Next row must be set when row lineage is enabled"); - } } diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadataSerialization.java b/core/src/test/java/org/apache/iceberg/TestTableMetadataSerialization.java index 63925aca1cc7..94a3d35b35a6 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadataSerialization.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadataSerialization.java @@ -72,7 +72,5 @@ public void testSerialization() throws Exception { assertThat(Lists.transform(result.snapshots(), Snapshot::snapshotId)) .isEqualTo(Lists.transform(meta.snapshots(), Snapshot::snapshotId)); assertThat(result.snapshotLog()).isEqualTo(meta.snapshotLog()); - assertThat(result.rowLinageEnabled()).isEqualTo(meta.rowLinageEnabled()); - assertThat(result.nextRowId()).isEqualTo(meta.nextRowId()); } } diff --git a/core/src/test/resources/TableMetadataV3MissingNextRowId.json b/core/src/test/resources/TableMetadataV3MissingNextRowId.json deleted file mode 100644 index a7e0ea96691d..000000000000 --- a/core/src/test/resources/TableMetadataV3MissingNextRowId.json +++ /dev/null @@ -1,161 +0,0 @@ -{ - "format-version": 3, - "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", - "location": "s3://bucket/test/location", - "last-sequence-number": 34, - "last-updated-ms": 1733710954170, - "last-column-id": 3, - "current-schema-id": 1, - "schemas": [ - { - "type": "struct", - "schema-id": 0, - "fields": [ - { - "id": 1, - "name": "x", - "required": true, - "type": "long" - } - ] - }, - { - "type": "struct", - "schema-id": 1, - "identifier-field-ids": [ - 1, - 2 - ], - "fields": [ - { - "id": 1, - "name": "x", - "required": true, - "type": "long" - }, - { - "id": 2, - "name": "y", - "required": true, - "type": "long", - "doc": "comment" - }, - { - "id": 3, - "name": "z", - "required": true, - "type": "long" - } - ] - }, - { - "type": "struct", - "schema-id": 6, - "fields": [ - { - "id": 10, - "name": "x", - "required": true, - "type": "string" - } - ] - } - ], - "default-spec-id": 5, - "partition-specs": [ - { - "spec-id": 5, - "fields": [] - } - ], - "last-partition-id": 999, - "default-sort-order-id": 3, - "sort-orders": [ - { - "order-id": 3, - "fields": [ - { - "transform": "identity", - "source-id": 2, - "direction": "asc", - "null-order": "nulls-first" - }, - { - "transform": "bucket[4]", - "source-id": 3, - "direction": "desc", - "null-order": "nulls-last" - } - ] - } - ], - "properties": { - "property": "value" - }, - "current-snapshot-id": 1733710954168, - "refs": { - "main": { - "snapshot-id": 1733710954168, - "type": "branch" - }, - "previous": { - "snapshot-id": 1733710953138, - "type": "tag" - }, - "test": { - "snapshot-id": 1733710953138, - "type": "branch" - } - }, - "snapshots": [ - { - "snapshot-id": 1733710953138, - "timestamp-ms": 1733710953138, - "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17078459248221091859.tmp" - }, - { - "snapshot-id": 1733710954168, - "parent-snapshot-id": 1733710953138, - "timestamp-ms": 1733710954168, - "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17490643917806818981.tmp", - "schema-id": 7 - } - ], - "statistics": [ - { - "snapshot-id": 11, - "statistics-path": "/some/stats/file.puffin", - "file-size-in-bytes": 100, - "file-footer-size-in-bytes": 42, - "blob-metadata": [ - { - "type": "some-stats", - "snapshot-id": 11, - "sequence-number": 2, - "fields": [ - 4 - ] - } - ] - } - ], - "partition-statistics": [ - { - "snapshot-id": 11, - "statistics-path": "/some/partition/stats/file.parquet", - "file-size-in-bytes": 42 - } - ], - "row-lineage": true, - "snapshot-log": [ - { - "timestamp-ms": 1733710953138, - "snapshot-id": 1733710953138 - }, - { - "timestamp-ms": 1733710954168, - "snapshot-id": 1733710954168 - } - ], - "metadata-log": [] -} \ No newline at end of file diff --git a/core/src/test/resources/TableMetadataV3RowLineageEnabled.json b/core/src/test/resources/TableMetadataV3RowLineageEnabled.json deleted file mode 100644 index 75ee92cca6a7..000000000000 --- a/core/src/test/resources/TableMetadataV3RowLineageEnabled.json +++ /dev/null @@ -1,162 +0,0 @@ -{ - "format-version": 3, - "table-uuid": "9faafb13-1ce7-4603-93f8-197afc7394a9", - "location": "s3://bucket/test/location", - "last-sequence-number": 34, - "last-updated-ms": 1733710954170, - "last-column-id": 3, - "current-schema-id": 1, - "schemas": [ - { - "type": "struct", - "schema-id": 0, - "fields": [ - { - "id": 1, - "name": "x", - "required": true, - "type": "long" - } - ] - }, - { - "type": "struct", - "schema-id": 1, - "identifier-field-ids": [ - 1, - 2 - ], - "fields": [ - { - "id": 1, - "name": "x", - "required": true, - "type": "long" - }, - { - "id": 2, - "name": "y", - "required": true, - "type": "long", - "doc": "comment" - }, - { - "id": 3, - "name": "z", - "required": true, - "type": "long" - } - ] - }, - { - "type": "struct", - "schema-id": 6, - "fields": [ - { - "id": 10, - "name": "x", - "required": true, - "type": "string" - } - ] - } - ], - "default-spec-id": 5, - "partition-specs": [ - { - "spec-id": 5, - "fields": [] - } - ], - "last-partition-id": 999, - "default-sort-order-id": 3, - "sort-orders": [ - { - "order-id": 3, - "fields": [ - { - "transform": "identity", - "source-id": 2, - "direction": "asc", - "null-order": "nulls-first" - }, - { - "transform": "bucket[4]", - "source-id": 3, - "direction": "desc", - "null-order": "nulls-last" - } - ] - } - ], - "properties": { - "property": "value" - }, - "current-snapshot-id": 1733710954168, - "refs": { - "main": { - "snapshot-id": 1733710954168, - "type": "branch" - }, - "previous": { - "snapshot-id": 1733710953138, - "type": "tag" - }, - "test": { - "snapshot-id": 1733710953138, - "type": "branch" - } - }, - "snapshots": [ - { - "snapshot-id": 1733710953138, - "timestamp-ms": 1733710953138, - "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17078459248221091859.tmp" - }, - { - "snapshot-id": 1733710954168, - "parent-snapshot-id": 1733710953138, - "timestamp-ms": 1733710954168, - "manifest-list": "/var/folders/z8/cv70q7pj1l13r_j628cyyks80000gn/T/junit-9477945946492951907/manifests17490643917806818981.tmp", - "schema-id": 7 - } - ], - "statistics": [ - { - "snapshot-id": 11, - "statistics-path": "/some/stats/file.puffin", - "file-size-in-bytes": 100, - "file-footer-size-in-bytes": 42, - "blob-metadata": [ - { - "type": "some-stats", - "snapshot-id": 11, - "sequence-number": 2, - "fields": [ - 4 - ] - } - ] - } - ], - "partition-statistics": [ - { - "snapshot-id": 11, - "statistics-path": "/some/partition/stats/file.parquet", - "file-size-in-bytes": 42 - } - ], - "row-lineage": true, - "next-row-id": 0, - "snapshot-log": [ - { - "timestamp-ms": 1733710953138, - "snapshot-id": 1733710953138 - }, - { - "timestamp-ms": 1733710954168, - "snapshot-id": 1733710954168 - } - ], - "metadata-log": [] -} \ No newline at end of file From 20c72d0d37c4663966c5ec2e8c521ba1ee6f9e0a Mon Sep 17 00:00:00 2001 From: Honah J Date: Fri, 10 Jan 2025 16:11:30 -0800 Subject: [PATCH 08/15] First try: Add Metadata Builder for tests --- .../org/apache/iceberg/MetadataTestUtils.java | 239 +++++++ .../org/apache/iceberg/TestTableMetadata.java | 610 +++++++++--------- 2 files changed, 549 insertions(+), 300 deletions(-) create mode 100644 core/src/test/java/org/apache/iceberg/MetadataTestUtils.java diff --git a/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java b/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java new file mode 100644 index 000000000000..cd59d56fdbb9 --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java @@ -0,0 +1,239 @@ +/* + * 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 static org.apache.iceberg.TableMetadata.INITIAL_SEQUENCE_NUMBER; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import org.apache.iceberg.relocated.com.google.common.collect.Lists; +import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.apache.iceberg.util.SerializableSupplier; + +public class MetadataTestUtils { + + public static TableMetadataBuilder buildTestTableMetadataFromEmpty(int formatVersion) { + return new TableMetadataBuilder(formatVersion); + } + + public static class TableMetadataBuilder { + private String metadataLocation; + private int formatVersion; + private String uuid; + private String location; + private long lastSequenceNumber; + private Long lastUpdatedMillis; + private int lastColumnId; + private int currentSchemaId; + private List schemas; + private int defaultSpecId; + private List specs; + private int lastAssignedPartitionId; + private int defaultSortOrderId; + private List sortOrders; + private Map properties; + private long currentSnapshotId; + private List snapshotLog; + private List previousFiles; + private List statisticsFiles; + private List partitionStatisticsFiles; + private List changes; + private SerializableSupplier> snapshotsSupplier; + private List snapshots; + private Map refs; + + private TableMetadataBuilder(int formatVersion) { + this.formatVersion = formatVersion; + this.uuid = UUID.randomUUID().toString(); + this.lastSequenceNumber = INITIAL_SEQUENCE_NUMBER; + this.lastUpdatedMillis = System.currentTimeMillis(); + this.lastColumnId = -1; + this.currentSchemaId = -1; + this.schemas = Lists.newArrayList(); + this.defaultSpecId = -1; + this.specs = Lists.newArrayList(); + this.lastAssignedPartitionId = 999; + this.defaultSortOrderId = -1; + this.sortOrders = Lists.newArrayList(); + this.properties = Maps.newHashMap(); + this.snapshots = Lists.newArrayList(); + this.currentSnapshotId = -1; + this.changes = Lists.newArrayList(); + this.snapshotLog = Lists.newArrayList(); + this.previousFiles = Lists.newArrayList(); + this.refs = Maps.newHashMap(); + this.statisticsFiles = Lists.newArrayList(); + this.partitionStatisticsFiles = Lists.newArrayList(); + } + + public TableMetadataBuilder withMetadataFileLocation(String metadataFileLocation) { + this.metadataLocation = metadataFileLocation; + return this; + } + + public TableMetadataBuilder withFormatVersion(int formatVersion) { + this.formatVersion = formatVersion; + return this; + } + + public TableMetadataBuilder withUUID(String uuid) { + this.uuid = uuid; + return this; + } + + public TableMetadataBuilder withLocation(String location) { + this.location = location; + return this; + } + + public TableMetadataBuilder withLastSequenceNumber(long lastSequenceNumber) { + this.lastSequenceNumber = lastSequenceNumber; + return this; + } + + public TableMetadataBuilder withLastUpdatedMillis(long lastUpdatedMillis) { + this.lastUpdatedMillis = lastUpdatedMillis; + return this; + } + + public TableMetadataBuilder withLastColumnId(int lastColumnId) { + this.lastColumnId = lastColumnId; + return this; + } + + public TableMetadataBuilder withCurrentSchemaId(int currentSchemaId) { + this.currentSchemaId = currentSchemaId; + return this; + } + + public TableMetadataBuilder withSchemas(List schemas) { + this.schemas = schemas; + return this; + } + + public TableMetadataBuilder withDefaultSpecId(int defaultSpecId) { + this.defaultSpecId = defaultSpecId; + return this; + } + + public TableMetadataBuilder withSpecs(List specs) { + this.specs = specs; + return this; + } + + public TableMetadataBuilder withLastAssignedPartitionId(int lastAssignedPartitionId) { + this.lastAssignedPartitionId = lastAssignedPartitionId; + return this; + } + + public TableMetadataBuilder withDefaultSortOrderId(int defaultSortOrderId) { + this.defaultSortOrderId = defaultSortOrderId; + return this; + } + + public TableMetadataBuilder withSortOrders(List sortOrders) { + this.sortOrders = sortOrders; + return this; + } + + public TableMetadataBuilder withProperties(Map properties) { + this.properties = properties; + return this; + } + + public TableMetadataBuilder withCurrentSnapshotId(long snapshotId) { + this.currentSnapshotId = snapshotId; + return this; + } + + public TableMetadataBuilder withSnapshotsSupplier( + SerializableSupplier> snapshotsSupplier) { + this.snapshotsSupplier = snapshotsSupplier; + return this; + } + + public TableMetadataBuilder withSnapshots(List snapshots) { + this.snapshots = snapshots; + return this; + } + + public TableMetadataBuilder withSnapshotLog(List snapshotLog) { + this.snapshotLog = snapshotLog; + return this; + } + + public TableMetadataBuilder withMetadataHistory( + List previousFiles) { + this.previousFiles = previousFiles; + return this; + } + + public TableMetadataBuilder withRefs(Map refs) { + this.refs = refs; + return this; + } + + public TableMetadataBuilder withChanges(List changes) { + this.changes = changes; + return this; + } + + public TableMetadataBuilder withStatisticsFiles(List statisticsFiles) { + this.statisticsFiles = statisticsFiles; + return this; + } + + public TableMetadataBuilder withPartitionStatisticsFiles( + List partitionStatisticsFiles) { + this.partitionStatisticsFiles = partitionStatisticsFiles; + return this; + } + + public TableMetadata build() { + return new TableMetadata( + metadataLocation, + formatVersion, + uuid, + location, + lastSequenceNumber, + lastUpdatedMillis, + lastColumnId, + currentSchemaId, + ImmutableList.copyOf(schemas), + defaultSpecId, + ImmutableList.copyOf(specs), + lastAssignedPartitionId, + defaultSortOrderId, + ImmutableList.copyOf(sortOrders), + ImmutableMap.copyOf(properties), + currentSnapshotId, + ImmutableList.copyOf(snapshots), + snapshotsSupplier, + ImmutableList.copyOf(snapshotLog), + ImmutableList.copyOf(previousFiles), + ImmutableMap.copyOf(refs), + ImmutableList.copyOf(statisticsFiles), + ImmutableList.copyOf(partitionStatisticsFiles), + ImmutableList.copyOf(changes)); + } + } +} diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index a593f3537a01..87482d787979 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -168,31 +168,32 @@ public void testJsonConversion(int formatVersion) throws Exception { .build()); TableMetadata expected = - new TableMetadata( - null, - formatVersion, - UUID.randomUUID().toString(), - TEST_LOCATION, - SEQ_NO, - System.currentTimeMillis(), - 3, - 7, - ImmutableList.of(TEST_SCHEMA, schema), - 5, - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of("property", "value"), - currentSnapshotId, - Arrays.asList(previousSnapshot, currentSnapshot), - null, - snapshotLog, - ImmutableList.of(), - refs, - statisticsFiles, - partitionStatisticsFiles, - ImmutableList.of()); + MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + .withMetadataFileLocation(null) + .withFormatVersion(formatVersion) + .withUUID(UUID.randomUUID().toString()) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(SEQ_NO) + .withLastUpdatedMillis(System.currentTimeMillis()) + .withLastColumnId(3) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA, schema)) + .withDefaultSpecId(5) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(3) + .withSortOrders(ImmutableList.of(SORT_ORDER_3)) + .withProperties(ImmutableMap.of("property", "value")) + .withCurrentSnapshotId(currentSnapshotId) + .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .withSnapshotsSupplier(null) + .withSnapshotLog(snapshotLog) + .withMetadataHistory(ImmutableList.of()) + .withRefs(refs) + .withStatisticsFiles(statisticsFiles) + .withPartitionStatisticsFiles(partitionStatisticsFiles) + .withChanges(ImmutableList.of()) + .build(); String asJson = TableMetadataParser.toJson(expected); TableMetadata metadata = TableMetadataParser.fromJson(asJson); @@ -257,31 +258,32 @@ public void testBackwardCompat() throws Exception { manifestList); TableMetadata expected = - new TableMetadata( - null, - 1, - null, - TEST_LOCATION, - 0, - System.currentTimeMillis(), - 3, - TableMetadata.INITIAL_SCHEMA_ID, - ImmutableList.of(schema), - 6, - ImmutableList.of(spec), - spec.lastAssignedFieldId(), - TableMetadata.INITIAL_SORT_ORDER_ID, - ImmutableList.of(sortOrder), - ImmutableMap.of("property", "value"), - currentSnapshotId, - Arrays.asList(previousSnapshot, currentSnapshot), - null, - ImmutableList.of(), - ImmutableList.of(), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of()); + MetadataTestUtils.buildTestTableMetadataFromEmpty(1) + .withMetadataFileLocation(null) + .withFormatVersion(1) + .withUUID(null) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(0) + .withLastUpdatedMillis(System.currentTimeMillis()) + .withLastColumnId(3) + .withCurrentSchemaId(TableMetadata.INITIAL_SCHEMA_ID) + .withSchemas(ImmutableList.of(schema)) + .withDefaultSpecId(6) + .withSpecs(ImmutableList.of(spec)) + .withLastAssignedPartitionId(spec.lastAssignedFieldId()) + .withDefaultSortOrderId(TableMetadata.INITIAL_SORT_ORDER_ID) + .withSortOrders(ImmutableList.of(sortOrder)) + .withProperties(ImmutableMap.of("property", "value")) + .withCurrentSnapshotId(currentSnapshotId) + .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .withSnapshotsSupplier(null) + .withSnapshotLog(ImmutableList.of()) + .withMetadataHistory(ImmutableList.of()) + .withRefs(ImmutableMap.of()) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build(); String asJson = toJsonWithoutSpecAndSchemaList(expected); TableMetadata metadata = TableMetadataParser.fromJson(asJson); @@ -368,31 +370,32 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { assertThatThrownBy( () -> - new TableMetadata( - null, - formatVersion, - UUID.randomUUID().toString(), - TEST_LOCATION, - SEQ_NO, - System.currentTimeMillis(), - 3, - 7, - ImmutableList.of(TEST_SCHEMA, schema), - 5, - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of("property", "value"), - currentSnapshotId, - Arrays.asList(previousSnapshot, currentSnapshot), - null, - snapshotLog, - ImmutableList.of(), - refs, - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of())) + MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + .withMetadataFileLocation(null) + .withFormatVersion(formatVersion) + .withUUID(UUID.randomUUID().toString()) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(SEQ_NO) + .withLastUpdatedMillis(System.currentTimeMillis()) + .withLastColumnId(3) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA, schema)) + .withDefaultSpecId(5) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(3) + .withSortOrders(ImmutableList.of(SORT_ORDER_3)) + .withProperties(ImmutableMap.of("property", "value")) + .withCurrentSnapshotId(currentSnapshotId) + .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .withSnapshotsSupplier(null) + .withSnapshotLog(snapshotLog) + .withMetadataHistory(ImmutableList.of()) + .withRefs(refs) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Current snapshot ID does not match main branch"); } @@ -416,31 +419,31 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { assertThatThrownBy( () -> - new TableMetadata( - null, - formatVersion, - UUID.randomUUID().toString(), - TEST_LOCATION, - SEQ_NO, - System.currentTimeMillis(), - 3, - 7, - ImmutableList.of(TEST_SCHEMA, schema), - 5, - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of("property", "value"), - -1, - ImmutableList.of(snapshot), - null, - ImmutableList.of(), - ImmutableList.of(), - refs, - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of())) + MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + .withMetadataFileLocation(null) + .withFormatVersion(formatVersion) + .withUUID(UUID.randomUUID().toString()) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(SEQ_NO) + .withLastUpdatedMillis(System.currentTimeMillis()) + .withLastColumnId(3) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA, schema)) + .withDefaultSpecId(5) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(3) + .withSortOrders(ImmutableList.of(SORT_ORDER_3)) + .withProperties(ImmutableMap.of("property", "value")) + .withCurrentSnapshotId(-1) + .withSnapshots(ImmutableList.of(snapshot)) + .withSnapshotsSupplier(null) + .withMetadataHistory(ImmutableList.of()) + .withRefs(refs) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Current snapshot is not set, but main branch exists"); } @@ -459,31 +462,31 @@ public void testBranchSnapshotMissing(int formatVersion) { assertThatThrownBy( () -> - new TableMetadata( - null, - formatVersion, - UUID.randomUUID().toString(), - TEST_LOCATION, - SEQ_NO, - System.currentTimeMillis(), - 3, - 7, - ImmutableList.of(TEST_SCHEMA, schema), - 5, - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of("property", "value"), - -1, - ImmutableList.of(), - null, - ImmutableList.of(), - ImmutableList.of(), - refs, - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of())) + MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + .withMetadataFileLocation(null) + .withFormatVersion(formatVersion) + .withUUID(UUID.randomUUID().toString()) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(SEQ_NO) + .withLastUpdatedMillis(System.currentTimeMillis()) + .withLastColumnId(3) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA, schema)) + .withDefaultSpecId(5) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(3) + .withSortOrders(ImmutableList.of(SORT_ORDER_3)) + .withProperties(ImmutableMap.of("property", "value")) + .withCurrentSnapshotId(-1) + .withSnapshots(ImmutableList.of()) + .withSnapshotsSupplier(null) + .withMetadataHistory(ImmutableList.of()) + .withRefs(refs) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageEndingWith("does not exist in the existing snapshots list"); } @@ -564,31 +567,32 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception currentTimestamp, "/tmp/000001-" + UUID.randomUUID() + ".metadata.json")); TableMetadata base = - new TableMetadata( - null, - formatVersion, - UUID.randomUUID().toString(), - TEST_LOCATION, - 0, - System.currentTimeMillis(), - 3, - 7, - ImmutableList.of(TEST_SCHEMA), - 5, - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of("property", "value"), - currentSnapshotId, - Arrays.asList(previousSnapshot, currentSnapshot), - null, - reversedSnapshotLog, - ImmutableList.copyOf(previousMetadataLog), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of()); + MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + .withMetadataFileLocation(null) + .withFormatVersion(formatVersion) + .withUUID(UUID.randomUUID().toString()) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(0) + .withLastUpdatedMillis(System.currentTimeMillis()) + .withLastColumnId(3) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA)) + .withDefaultSpecId(5) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(3) + .withSortOrders(ImmutableList.of(SORT_ORDER_3)) + .withProperties(ImmutableMap.of("property", "value")) + .withCurrentSnapshotId(currentSnapshotId) + .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .withSnapshotsSupplier(null) + .withSnapshotLog(reversedSnapshotLog) + .withMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) + .withRefs(ImmutableMap.of()) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build(); String asJson = TableMetadataParser.toJson(base); TableMetadata metadataFromJson = TableMetadataParser.fromJson(asJson); @@ -641,31 +645,32 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept currentTimestamp - 80, "/tmp/000003-" + UUID.randomUUID() + ".metadata.json"); TableMetadata base = - new TableMetadata( - latestPreviousMetadata.file(), - formatVersion, - UUID.randomUUID().toString(), - TEST_LOCATION, - 0, - currentTimestamp - 80, - 3, - 7, - ImmutableList.of(TEST_SCHEMA), - 5, - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of("property", "value"), - currentSnapshotId, - Arrays.asList(previousSnapshot, currentSnapshot), - null, - reversedSnapshotLog, - ImmutableList.copyOf(previousMetadataLog), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of()); + MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + .withMetadataFileLocation(latestPreviousMetadata.file()) + .withFormatVersion(formatVersion) + .withUUID(UUID.randomUUID().toString()) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(0) + .withLastUpdatedMillis(currentTimestamp - 80) + .withLastColumnId(3) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA)) + .withDefaultSpecId(5) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(3) + .withSortOrders(ImmutableList.of(SORT_ORDER_3)) + .withProperties(ImmutableMap.of("property", "value")) + .withCurrentSnapshotId(currentSnapshotId) + .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .withSnapshotsSupplier(null) + .withSnapshotLog(reversedSnapshotLog) + .withMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) + .withRefs(ImmutableMap.of()) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build(); previousMetadataLog.add(latestPreviousMetadata); @@ -733,31 +738,32 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti currentTimestamp - 50, "/tmp/000006-" + UUID.randomUUID() + ".metadata.json"); TableMetadata base = - new TableMetadata( - latestPreviousMetadata.file(), - formatVersion, - UUID.randomUUID().toString(), - TEST_LOCATION, - 0, - currentTimestamp - 50, - 3, - 7, - ImmutableList.of(TEST_SCHEMA), - 5, - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of("property", "value"), - currentSnapshotId, - Arrays.asList(previousSnapshot, currentSnapshot), - null, - reversedSnapshotLog, - ImmutableList.copyOf(previousMetadataLog), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of()); + MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + .withMetadataFileLocation(latestPreviousMetadata.file()) + .withFormatVersion(formatVersion) + .withUUID(UUID.randomUUID().toString()) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(0) + .withLastUpdatedMillis(currentTimestamp - 50) + .withLastColumnId(3) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA)) + .withDefaultSpecId(5) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(3) + .withSortOrders(ImmutableList.of(SORT_ORDER_3)) + .withProperties(ImmutableMap.of("property", "value")) + .withCurrentSnapshotId(currentSnapshotId) + .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .withSnapshotsSupplier(null) + .withSnapshotLog(reversedSnapshotLog) + .withMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) + .withRefs(ImmutableMap.of()) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build(); previousMetadataLog.add(latestPreviousMetadata); @@ -829,31 +835,32 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx currentTimestamp - 50, "/tmp/000006-" + UUID.randomUUID() + ".metadata.json"); TableMetadata base = - new TableMetadata( - latestPreviousMetadata.file(), - formatVersion, - UUID.randomUUID().toString(), - TEST_LOCATION, - 0, - currentTimestamp - 50, - 3, - 7, - ImmutableList.of(TEST_SCHEMA), - SPEC_5.specId(), - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - SortOrder.unsorted().orderId(), - ImmutableList.of(SortOrder.unsorted()), - ImmutableMap.of("property", "value"), - currentSnapshotId, - Arrays.asList(previousSnapshot, currentSnapshot), - null, - reversedSnapshotLog, - ImmutableList.copyOf(previousMetadataLog), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of()); + MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + .withMetadataFileLocation(latestPreviousMetadata.file()) + .withFormatVersion(formatVersion) + .withUUID(UUID.randomUUID().toString()) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(0) + .withLastUpdatedMillis(currentTimestamp - 50) + .withLastColumnId(3) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA)) + .withDefaultSpecId(SPEC_5.specId()) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(SortOrder.unsorted().orderId()) + .withSortOrders(ImmutableList.of(SortOrder.unsorted())) + .withProperties(ImmutableMap.of("property", "value")) + .withCurrentSnapshotId(currentSnapshotId) + .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .withSnapshotsSupplier(null) + .withSnapshotLog(reversedSnapshotLog) + .withMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) + .withRefs(ImmutableMap.of()) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build(); previousMetadataLog.add(latestPreviousMetadata); @@ -878,31 +885,32 @@ public void testV2UUIDValidation(int formatVersion) { assertThatThrownBy( () -> - new TableMetadata( - null, - formatVersion, - null, - TEST_LOCATION, - SEQ_NO, - System.currentTimeMillis(), - LAST_ASSIGNED_COLUMN_ID, - 7, - ImmutableList.of(TEST_SCHEMA), - SPEC_5.specId(), - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of(), - -1L, - ImmutableList.of(), - null, - ImmutableList.of(), - ImmutableList.of(), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of())) + MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + .withMetadataFileLocation(null) + .withFormatVersion(formatVersion) + .withUUID(null) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(SEQ_NO) + .withLastUpdatedMillis(System.currentTimeMillis()) + .withLastColumnId(LAST_ASSIGNED_COLUMN_ID) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA)) + .withDefaultSpecId(SPEC_5.specId()) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(3) + .withSortOrders(ImmutableList.of(SORT_ORDER_3)) + .withProperties(ImmutableMap.of()) + .withCurrentSnapshotId(-1L) + .withSnapshots(ImmutableList.of()) + .withSnapshotsSupplier(null) + .withSnapshotLog(ImmutableList.of()) + .withMetadataHistory(ImmutableList.of()) + .withRefs(ImmutableMap.of()) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessage(String.format("UUID is required in format v%s", formatVersion)); } @@ -913,31 +921,32 @@ public void testVersionValidation() { int unsupportedVersion = supportedVersion + 1; assertThatThrownBy( () -> - new TableMetadata( - null, - unsupportedVersion, - null, - TEST_LOCATION, - SEQ_NO, - System.currentTimeMillis(), - LAST_ASSIGNED_COLUMN_ID, - 7, - ImmutableList.of(TEST_SCHEMA), - SPEC_5.specId(), - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of(), - -1L, - ImmutableList.of(), - null, - ImmutableList.of(), - ImmutableList.of(), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of())) + MetadataTestUtils.buildTestTableMetadataFromEmpty(unsupportedVersion) + .withMetadataFileLocation(null) + .withFormatVersion(unsupportedVersion) + .withUUID(null) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(SEQ_NO) + .withLastUpdatedMillis(System.currentTimeMillis()) + .withLastColumnId(LAST_ASSIGNED_COLUMN_ID) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA)) + .withDefaultSpecId(SPEC_5.specId()) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(3) + .withSortOrders(ImmutableList.of(SORT_ORDER_3)) + .withProperties(ImmutableMap.of()) + .withCurrentSnapshotId(-1L) + .withSnapshots(ImmutableList.of()) + .withSnapshotsSupplier(null) + .withSnapshotLog(ImmutableList.of()) + .withMetadataHistory(ImmutableList.of()) + .withRefs(ImmutableMap.of()) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessage( "Unsupported format version: v%s (supported: v%s)", @@ -959,31 +968,32 @@ public void testVersionValidation() { // should be allowed in the supported version assertThat( - new TableMetadata( - null, - supportedVersion, - UUID.randomUUID().toString(), - TEST_LOCATION, - SEQ_NO, - System.currentTimeMillis(), - LAST_ASSIGNED_COLUMN_ID, - 7, - ImmutableList.of(TEST_SCHEMA), - SPEC_5.specId(), - ImmutableList.of(SPEC_5), - SPEC_5.lastAssignedFieldId(), - 3, - ImmutableList.of(SORT_ORDER_3), - ImmutableMap.of(), - -1L, - ImmutableList.of(), - null, - ImmutableList.of(), - ImmutableList.of(), - ImmutableMap.of(), - ImmutableList.of(), - ImmutableList.of(), - ImmutableList.of())) + MetadataTestUtils.buildTestTableMetadataFromEmpty(supportedVersion) + .withMetadataFileLocation(null) + .withFormatVersion(supportedVersion) + .withUUID(UUID.randomUUID().toString()) + .withLocation(TEST_LOCATION) + .withLastSequenceNumber(SEQ_NO) + .withLastUpdatedMillis(System.currentTimeMillis()) + .withLastColumnId(LAST_ASSIGNED_COLUMN_ID) + .withCurrentSchemaId(7) + .withSchemas(ImmutableList.of(TEST_SCHEMA)) + .withDefaultSpecId(SPEC_5.specId()) + .withSpecs(ImmutableList.of(SPEC_5)) + .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .withDefaultSortOrderId(3) + .withSortOrders(ImmutableList.of(SORT_ORDER_3)) + .withProperties(ImmutableMap.of()) + .withCurrentSnapshotId(-1L) + .withSnapshots(ImmutableList.of()) + .withSnapshotsSupplier(null) + .withSnapshotLog(ImmutableList.of()) + .withMetadataHistory(ImmutableList.of()) + .withRefs(ImmutableMap.of()) + .withStatisticsFiles(ImmutableList.of()) + .withPartitionStatisticsFiles(ImmutableList.of()) + .withChanges(ImmutableList.of()) + .build()) .isNotNull(); assertThat( From 4adb693625a45a9926bfe71296b1aac3dfbb3a50 Mon Sep 17 00:00:00 2001 From: Honah J Date: Fri, 10 Jan 2025 20:55:53 -0800 Subject: [PATCH 09/15] fix style issue --- .../org/apache/iceberg/MetadataTestUtils.java | 58 +- .../org/apache/iceberg/TestTableMetadata.java | 572 +++++++++--------- 2 files changed, 316 insertions(+), 314 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java b/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java index cd59d56fdbb9..f3bb86616201 100644 --- a/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java +++ b/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java @@ -20,17 +20,19 @@ import static org.apache.iceberg.TableMetadata.INITIAL_SEQUENCE_NUMBER; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import java.util.List; import java.util.Map; import java.util.UUID; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; +import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.apache.iceberg.relocated.com.google.common.collect.Maps; import org.apache.iceberg.util.SerializableSupplier; public class MetadataTestUtils { + private MetadataTestUtils() {} + public static TableMetadataBuilder buildTestTableMetadataFromEmpty(int formatVersion) { return new TableMetadataBuilder(formatVersion); } @@ -85,124 +87,124 @@ private TableMetadataBuilder(int formatVersion) { this.partitionStatisticsFiles = Lists.newArrayList(); } - public TableMetadataBuilder withMetadataFileLocation(String metadataFileLocation) { + public TableMetadataBuilder setMetadataLocation(String metadataFileLocation) { this.metadataLocation = metadataFileLocation; return this; } - public TableMetadataBuilder withFormatVersion(int formatVersion) { + public TableMetadataBuilder setFormatVersion(int formatVersion) { this.formatVersion = formatVersion; return this; } - public TableMetadataBuilder withUUID(String uuid) { + public TableMetadataBuilder setUuid(String uuid) { this.uuid = uuid; return this; } - public TableMetadataBuilder withLocation(String location) { + public TableMetadataBuilder setLocation(String location) { this.location = location; return this; } - public TableMetadataBuilder withLastSequenceNumber(long lastSequenceNumber) { + public TableMetadataBuilder setLastSequenceNumber(long lastSequenceNumber) { this.lastSequenceNumber = lastSequenceNumber; return this; } - public TableMetadataBuilder withLastUpdatedMillis(long lastUpdatedMillis) { + public TableMetadataBuilder setLastUpdatedMillis(long lastUpdatedMillis) { this.lastUpdatedMillis = lastUpdatedMillis; return this; } - public TableMetadataBuilder withLastColumnId(int lastColumnId) { + public TableMetadataBuilder setLastColumnId(int lastColumnId) { this.lastColumnId = lastColumnId; return this; } - public TableMetadataBuilder withCurrentSchemaId(int currentSchemaId) { + public TableMetadataBuilder setCurrentSchemaId(int currentSchemaId) { this.currentSchemaId = currentSchemaId; return this; } - public TableMetadataBuilder withSchemas(List schemas) { + public TableMetadataBuilder setSchemas(List schemas) { this.schemas = schemas; return this; } - public TableMetadataBuilder withDefaultSpecId(int defaultSpecId) { + public TableMetadataBuilder setDefaultSpecId(int defaultSpecId) { this.defaultSpecId = defaultSpecId; return this; } - public TableMetadataBuilder withSpecs(List specs) { + public TableMetadataBuilder setSpecs(List specs) { this.specs = specs; return this; } - public TableMetadataBuilder withLastAssignedPartitionId(int lastAssignedPartitionId) { + public TableMetadataBuilder setLastAssignedPartitionId(int lastAssignedPartitionId) { this.lastAssignedPartitionId = lastAssignedPartitionId; return this; } - public TableMetadataBuilder withDefaultSortOrderId(int defaultSortOrderId) { + public TableMetadataBuilder setDefaultSortOrderId(int defaultSortOrderId) { this.defaultSortOrderId = defaultSortOrderId; return this; } - public TableMetadataBuilder withSortOrders(List sortOrders) { + public TableMetadataBuilder setSortOrders(List sortOrders) { this.sortOrders = sortOrders; return this; } - public TableMetadataBuilder withProperties(Map properties) { + public TableMetadataBuilder setProperties(Map properties) { this.properties = properties; return this; } - public TableMetadataBuilder withCurrentSnapshotId(long snapshotId) { + public TableMetadataBuilder setCurrentSnapshotId(long snapshotId) { this.currentSnapshotId = snapshotId; return this; } - public TableMetadataBuilder withSnapshotsSupplier( + public TableMetadataBuilder setSnapshotsSupplier( SerializableSupplier> snapshotsSupplier) { this.snapshotsSupplier = snapshotsSupplier; return this; } - public TableMetadataBuilder withSnapshots(List snapshots) { + public TableMetadataBuilder setSnapshots(List snapshots) { this.snapshots = snapshots; return this; } - public TableMetadataBuilder withSnapshotLog(List snapshotLog) { + public TableMetadataBuilder setSnapshotLog(List snapshotLog) { this.snapshotLog = snapshotLog; return this; } - public TableMetadataBuilder withMetadataHistory( - List previousFiles) { - this.previousFiles = previousFiles; + public TableMetadataBuilder setMetadataHistory( + List metadataHistory) { + this.previousFiles = metadataHistory; return this; } - public TableMetadataBuilder withRefs(Map refs) { + public TableMetadataBuilder setRefs(Map refs) { this.refs = refs; return this; } - public TableMetadataBuilder withChanges(List changes) { + public TableMetadataBuilder setChanges(List changes) { this.changes = changes; return this; } - public TableMetadataBuilder withStatisticsFiles(List statisticsFiles) { + public TableMetadataBuilder setStatisticsFiles(List statisticsFiles) { this.statisticsFiles = statisticsFiles; return this; } - public TableMetadataBuilder withPartitionStatisticsFiles( + public TableMetadataBuilder setPartitionStatisticsFiles( List partitionStatisticsFiles) { this.partitionStatisticsFiles = partitionStatisticsFiles; return this; diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 87482d787979..10b8e6915d4f 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -169,30 +169,30 @@ public void testJsonConversion(int formatVersion) throws Exception { TableMetadata expected = MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .withMetadataFileLocation(null) - .withFormatVersion(formatVersion) - .withUUID(UUID.randomUUID().toString()) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(SEQ_NO) - .withLastUpdatedMillis(System.currentTimeMillis()) - .withLastColumnId(3) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA, schema)) - .withDefaultSpecId(5) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(3) - .withSortOrders(ImmutableList.of(SORT_ORDER_3)) - .withProperties(ImmutableMap.of("property", "value")) - .withCurrentSnapshotId(currentSnapshotId) - .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .withSnapshotsSupplier(null) - .withSnapshotLog(snapshotLog) - .withMetadataHistory(ImmutableList.of()) - .withRefs(refs) - .withStatisticsFiles(statisticsFiles) - .withPartitionStatisticsFiles(partitionStatisticsFiles) - .withChanges(ImmutableList.of()) + .setMetadataLocation(null) + .setFormatVersion(formatVersion) + .setUuid(UUID.randomUUID().toString()) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(SEQ_NO) + .setLastUpdatedMillis(System.currentTimeMillis()) + .setLastColumnId(3) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) + .setDefaultSpecId(5) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(3) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of("property", "value")) + .setCurrentSnapshotId(currentSnapshotId) + .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .setSnapshotsSupplier(null) + .setSnapshotLog(snapshotLog) + .setMetadataHistory(ImmutableList.of()) + .setRefs(refs) + .setStatisticsFiles(statisticsFiles) + .setPartitionStatisticsFiles(partitionStatisticsFiles) + .setChanges(ImmutableList.of()) .build(); String asJson = TableMetadataParser.toJson(expected); @@ -259,30 +259,30 @@ public void testBackwardCompat() throws Exception { TableMetadata expected = MetadataTestUtils.buildTestTableMetadataFromEmpty(1) - .withMetadataFileLocation(null) - .withFormatVersion(1) - .withUUID(null) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(0) - .withLastUpdatedMillis(System.currentTimeMillis()) - .withLastColumnId(3) - .withCurrentSchemaId(TableMetadata.INITIAL_SCHEMA_ID) - .withSchemas(ImmutableList.of(schema)) - .withDefaultSpecId(6) - .withSpecs(ImmutableList.of(spec)) - .withLastAssignedPartitionId(spec.lastAssignedFieldId()) - .withDefaultSortOrderId(TableMetadata.INITIAL_SORT_ORDER_ID) - .withSortOrders(ImmutableList.of(sortOrder)) - .withProperties(ImmutableMap.of("property", "value")) - .withCurrentSnapshotId(currentSnapshotId) - .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .withSnapshotsSupplier(null) - .withSnapshotLog(ImmutableList.of()) - .withMetadataHistory(ImmutableList.of()) - .withRefs(ImmutableMap.of()) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(null) + .setFormatVersion(1) + .setUuid(null) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(0) + .setLastUpdatedMillis(System.currentTimeMillis()) + .setLastColumnId(3) + .setCurrentSchemaId(TableMetadata.INITIAL_SCHEMA_ID) + .setSchemas(ImmutableList.of(schema)) + .setDefaultSpecId(6) + .setSpecs(ImmutableList.of(spec)) + .setLastAssignedPartitionId(spec.lastAssignedFieldId()) + .setDefaultSortOrderId(TableMetadata.INITIAL_SORT_ORDER_ID) + .setSortOrders(ImmutableList.of(sortOrder)) + .setProperties(ImmutableMap.of("property", "value")) + .setCurrentSnapshotId(currentSnapshotId) + .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .setSnapshotsSupplier(null) + .setSnapshotLog(ImmutableList.of()) + .setMetadataHistory(ImmutableList.of()) + .setRefs(ImmutableMap.of()) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build(); String asJson = toJsonWithoutSpecAndSchemaList(expected); @@ -371,30 +371,30 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { assertThatThrownBy( () -> MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .withMetadataFileLocation(null) - .withFormatVersion(formatVersion) - .withUUID(UUID.randomUUID().toString()) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(SEQ_NO) - .withLastUpdatedMillis(System.currentTimeMillis()) - .withLastColumnId(3) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA, schema)) - .withDefaultSpecId(5) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(3) - .withSortOrders(ImmutableList.of(SORT_ORDER_3)) - .withProperties(ImmutableMap.of("property", "value")) - .withCurrentSnapshotId(currentSnapshotId) - .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .withSnapshotsSupplier(null) - .withSnapshotLog(snapshotLog) - .withMetadataHistory(ImmutableList.of()) - .withRefs(refs) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(null) + .setFormatVersion(formatVersion) + .setUuid(UUID.randomUUID().toString()) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(SEQ_NO) + .setLastUpdatedMillis(System.currentTimeMillis()) + .setLastColumnId(3) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) + .setDefaultSpecId(5) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(3) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of("property", "value")) + .setCurrentSnapshotId(currentSnapshotId) + .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .setSnapshotsSupplier(null) + .setSnapshotLog(snapshotLog) + .setMetadataHistory(ImmutableList.of()) + .setRefs(refs) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Current snapshot ID does not match main branch"); @@ -420,29 +420,29 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { assertThatThrownBy( () -> MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .withMetadataFileLocation(null) - .withFormatVersion(formatVersion) - .withUUID(UUID.randomUUID().toString()) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(SEQ_NO) - .withLastUpdatedMillis(System.currentTimeMillis()) - .withLastColumnId(3) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA, schema)) - .withDefaultSpecId(5) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(3) - .withSortOrders(ImmutableList.of(SORT_ORDER_3)) - .withProperties(ImmutableMap.of("property", "value")) - .withCurrentSnapshotId(-1) - .withSnapshots(ImmutableList.of(snapshot)) - .withSnapshotsSupplier(null) - .withMetadataHistory(ImmutableList.of()) - .withRefs(refs) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(null) + .setFormatVersion(formatVersion) + .setUuid(UUID.randomUUID().toString()) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(SEQ_NO) + .setLastUpdatedMillis(System.currentTimeMillis()) + .setLastColumnId(3) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) + .setDefaultSpecId(5) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(3) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of("property", "value")) + .setCurrentSnapshotId(-1) + .setSnapshots(ImmutableList.of(snapshot)) + .setSnapshotsSupplier(null) + .setMetadataHistory(ImmutableList.of()) + .setRefs(refs) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Current snapshot is not set, but main branch exists"); @@ -463,29 +463,29 @@ public void testBranchSnapshotMissing(int formatVersion) { assertThatThrownBy( () -> MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .withMetadataFileLocation(null) - .withFormatVersion(formatVersion) - .withUUID(UUID.randomUUID().toString()) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(SEQ_NO) - .withLastUpdatedMillis(System.currentTimeMillis()) - .withLastColumnId(3) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA, schema)) - .withDefaultSpecId(5) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(3) - .withSortOrders(ImmutableList.of(SORT_ORDER_3)) - .withProperties(ImmutableMap.of("property", "value")) - .withCurrentSnapshotId(-1) - .withSnapshots(ImmutableList.of()) - .withSnapshotsSupplier(null) - .withMetadataHistory(ImmutableList.of()) - .withRefs(refs) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(null) + .setFormatVersion(formatVersion) + .setUuid(UUID.randomUUID().toString()) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(SEQ_NO) + .setLastUpdatedMillis(System.currentTimeMillis()) + .setLastColumnId(3) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) + .setDefaultSpecId(5) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(3) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of("property", "value")) + .setCurrentSnapshotId(-1) + .setSnapshots(ImmutableList.of()) + .setSnapshotsSupplier(null) + .setMetadataHistory(ImmutableList.of()) + .setRefs(refs) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageEndingWith("does not exist in the existing snapshots list"); @@ -568,30 +568,30 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception TableMetadata base = MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .withMetadataFileLocation(null) - .withFormatVersion(formatVersion) - .withUUID(UUID.randomUUID().toString()) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(0) - .withLastUpdatedMillis(System.currentTimeMillis()) - .withLastColumnId(3) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA)) - .withDefaultSpecId(5) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(3) - .withSortOrders(ImmutableList.of(SORT_ORDER_3)) - .withProperties(ImmutableMap.of("property", "value")) - .withCurrentSnapshotId(currentSnapshotId) - .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .withSnapshotsSupplier(null) - .withSnapshotLog(reversedSnapshotLog) - .withMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) - .withRefs(ImmutableMap.of()) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(null) + .setFormatVersion(formatVersion) + .setUuid(UUID.randomUUID().toString()) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(0) + .setLastUpdatedMillis(System.currentTimeMillis()) + .setLastColumnId(3) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA)) + .setDefaultSpecId(5) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(3) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of("property", "value")) + .setCurrentSnapshotId(currentSnapshotId) + .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .setSnapshotsSupplier(null) + .setSnapshotLog(reversedSnapshotLog) + .setMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) + .setRefs(ImmutableMap.of()) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build(); String asJson = TableMetadataParser.toJson(base); @@ -646,30 +646,30 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept TableMetadata base = MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .withMetadataFileLocation(latestPreviousMetadata.file()) - .withFormatVersion(formatVersion) - .withUUID(UUID.randomUUID().toString()) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(0) - .withLastUpdatedMillis(currentTimestamp - 80) - .withLastColumnId(3) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA)) - .withDefaultSpecId(5) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(3) - .withSortOrders(ImmutableList.of(SORT_ORDER_3)) - .withProperties(ImmutableMap.of("property", "value")) - .withCurrentSnapshotId(currentSnapshotId) - .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .withSnapshotsSupplier(null) - .withSnapshotLog(reversedSnapshotLog) - .withMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) - .withRefs(ImmutableMap.of()) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(latestPreviousMetadata.file()) + .setFormatVersion(formatVersion) + .setUuid(UUID.randomUUID().toString()) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(0) + .setLastUpdatedMillis(currentTimestamp - 80) + .setLastColumnId(3) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA)) + .setDefaultSpecId(5) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(3) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of("property", "value")) + .setCurrentSnapshotId(currentSnapshotId) + .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .setSnapshotsSupplier(null) + .setSnapshotLog(reversedSnapshotLog) + .setMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) + .setRefs(ImmutableMap.of()) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build(); previousMetadataLog.add(latestPreviousMetadata); @@ -739,30 +739,30 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti TableMetadata base = MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .withMetadataFileLocation(latestPreviousMetadata.file()) - .withFormatVersion(formatVersion) - .withUUID(UUID.randomUUID().toString()) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(0) - .withLastUpdatedMillis(currentTimestamp - 50) - .withLastColumnId(3) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA)) - .withDefaultSpecId(5) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(3) - .withSortOrders(ImmutableList.of(SORT_ORDER_3)) - .withProperties(ImmutableMap.of("property", "value")) - .withCurrentSnapshotId(currentSnapshotId) - .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .withSnapshotsSupplier(null) - .withSnapshotLog(reversedSnapshotLog) - .withMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) - .withRefs(ImmutableMap.of()) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(latestPreviousMetadata.file()) + .setFormatVersion(formatVersion) + .setUuid(UUID.randomUUID().toString()) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(0) + .setLastUpdatedMillis(currentTimestamp - 50) + .setLastColumnId(3) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA)) + .setDefaultSpecId(5) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(3) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of("property", "value")) + .setCurrentSnapshotId(currentSnapshotId) + .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .setSnapshotsSupplier(null) + .setSnapshotLog(reversedSnapshotLog) + .setMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) + .setRefs(ImmutableMap.of()) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build(); previousMetadataLog.add(latestPreviousMetadata); @@ -836,30 +836,30 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx TableMetadata base = MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .withMetadataFileLocation(latestPreviousMetadata.file()) - .withFormatVersion(formatVersion) - .withUUID(UUID.randomUUID().toString()) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(0) - .withLastUpdatedMillis(currentTimestamp - 50) - .withLastColumnId(3) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA)) - .withDefaultSpecId(SPEC_5.specId()) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(SortOrder.unsorted().orderId()) - .withSortOrders(ImmutableList.of(SortOrder.unsorted())) - .withProperties(ImmutableMap.of("property", "value")) - .withCurrentSnapshotId(currentSnapshotId) - .withSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .withSnapshotsSupplier(null) - .withSnapshotLog(reversedSnapshotLog) - .withMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) - .withRefs(ImmutableMap.of()) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(latestPreviousMetadata.file()) + .setFormatVersion(formatVersion) + .setUuid(UUID.randomUUID().toString()) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(0) + .setLastUpdatedMillis(currentTimestamp - 50) + .setLastColumnId(3) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA)) + .setDefaultSpecId(SPEC_5.specId()) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(SortOrder.unsorted().orderId()) + .setSortOrders(ImmutableList.of(SortOrder.unsorted())) + .setProperties(ImmutableMap.of("property", "value")) + .setCurrentSnapshotId(currentSnapshotId) + .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) + .setSnapshotsSupplier(null) + .setSnapshotLog(reversedSnapshotLog) + .setMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) + .setRefs(ImmutableMap.of()) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build(); previousMetadataLog.add(latestPreviousMetadata); @@ -886,30 +886,30 @@ public void testV2UUIDValidation(int formatVersion) { assertThatThrownBy( () -> MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .withMetadataFileLocation(null) - .withFormatVersion(formatVersion) - .withUUID(null) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(SEQ_NO) - .withLastUpdatedMillis(System.currentTimeMillis()) - .withLastColumnId(LAST_ASSIGNED_COLUMN_ID) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA)) - .withDefaultSpecId(SPEC_5.specId()) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(3) - .withSortOrders(ImmutableList.of(SORT_ORDER_3)) - .withProperties(ImmutableMap.of()) - .withCurrentSnapshotId(-1L) - .withSnapshots(ImmutableList.of()) - .withSnapshotsSupplier(null) - .withSnapshotLog(ImmutableList.of()) - .withMetadataHistory(ImmutableList.of()) - .withRefs(ImmutableMap.of()) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(null) + .setFormatVersion(formatVersion) + .setUuid(null) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(SEQ_NO) + .setLastUpdatedMillis(System.currentTimeMillis()) + .setLastColumnId(LAST_ASSIGNED_COLUMN_ID) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA)) + .setDefaultSpecId(SPEC_5.specId()) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(3) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of()) + .setCurrentSnapshotId(-1L) + .setSnapshots(ImmutableList.of()) + .setSnapshotsSupplier(null) + .setSnapshotLog(ImmutableList.of()) + .setMetadataHistory(ImmutableList.of()) + .setRefs(ImmutableMap.of()) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessage(String.format("UUID is required in format v%s", formatVersion)); @@ -922,30 +922,30 @@ public void testVersionValidation() { assertThatThrownBy( () -> MetadataTestUtils.buildTestTableMetadataFromEmpty(unsupportedVersion) - .withMetadataFileLocation(null) - .withFormatVersion(unsupportedVersion) - .withUUID(null) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(SEQ_NO) - .withLastUpdatedMillis(System.currentTimeMillis()) - .withLastColumnId(LAST_ASSIGNED_COLUMN_ID) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA)) - .withDefaultSpecId(SPEC_5.specId()) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(3) - .withSortOrders(ImmutableList.of(SORT_ORDER_3)) - .withProperties(ImmutableMap.of()) - .withCurrentSnapshotId(-1L) - .withSnapshots(ImmutableList.of()) - .withSnapshotsSupplier(null) - .withSnapshotLog(ImmutableList.of()) - .withMetadataHistory(ImmutableList.of()) - .withRefs(ImmutableMap.of()) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(null) + .setFormatVersion(unsupportedVersion) + .setUuid(null) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(SEQ_NO) + .setLastUpdatedMillis(System.currentTimeMillis()) + .setLastColumnId(LAST_ASSIGNED_COLUMN_ID) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA)) + .setDefaultSpecId(SPEC_5.specId()) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(3) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of()) + .setCurrentSnapshotId(-1L) + .setSnapshots(ImmutableList.of()) + .setSnapshotsSupplier(null) + .setSnapshotLog(ImmutableList.of()) + .setMetadataHistory(ImmutableList.of()) + .setRefs(ImmutableMap.of()) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessage( @@ -969,30 +969,30 @@ public void testVersionValidation() { // should be allowed in the supported version assertThat( MetadataTestUtils.buildTestTableMetadataFromEmpty(supportedVersion) - .withMetadataFileLocation(null) - .withFormatVersion(supportedVersion) - .withUUID(UUID.randomUUID().toString()) - .withLocation(TEST_LOCATION) - .withLastSequenceNumber(SEQ_NO) - .withLastUpdatedMillis(System.currentTimeMillis()) - .withLastColumnId(LAST_ASSIGNED_COLUMN_ID) - .withCurrentSchemaId(7) - .withSchemas(ImmutableList.of(TEST_SCHEMA)) - .withDefaultSpecId(SPEC_5.specId()) - .withSpecs(ImmutableList.of(SPEC_5)) - .withLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .withDefaultSortOrderId(3) - .withSortOrders(ImmutableList.of(SORT_ORDER_3)) - .withProperties(ImmutableMap.of()) - .withCurrentSnapshotId(-1L) - .withSnapshots(ImmutableList.of()) - .withSnapshotsSupplier(null) - .withSnapshotLog(ImmutableList.of()) - .withMetadataHistory(ImmutableList.of()) - .withRefs(ImmutableMap.of()) - .withStatisticsFiles(ImmutableList.of()) - .withPartitionStatisticsFiles(ImmutableList.of()) - .withChanges(ImmutableList.of()) + .setMetadataLocation(null) + .setFormatVersion(supportedVersion) + .setUuid(UUID.randomUUID().toString()) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(SEQ_NO) + .setLastUpdatedMillis(System.currentTimeMillis()) + .setLastColumnId(LAST_ASSIGNED_COLUMN_ID) + .setCurrentSchemaId(7) + .setSchemas(ImmutableList.of(TEST_SCHEMA)) + .setDefaultSpecId(SPEC_5.specId()) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(3) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of()) + .setCurrentSnapshotId(-1L) + .setSnapshots(ImmutableList.of()) + .setSnapshotsSupplier(null) + .setSnapshotLog(ImmutableList.of()) + .setMetadataHistory(ImmutableList.of()) + .setRefs(ImmutableMap.of()) + .setStatisticsFiles(ImmutableList.of()) + .setPartitionStatisticsFiles(ImmutableList.of()) + .setChanges(ImmutableList.of()) .build()) .isNotNull(); From 32ffbfd52aa0ee1f80175074db1e91d185fc4cf6 Mon Sep 17 00:00:00 2001 From: Honah J Date: Fri, 10 Jan 2025 21:20:33 -0800 Subject: [PATCH 10/15] simplify construction of table metadata in tests --- .../apache/iceberg/TableMetadataParser.java | 2 - .../org/apache/iceberg/TestTableMetadata.java | 98 ------------------- 2 files changed, 100 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/TableMetadataParser.java b/core/src/main/java/org/apache/iceberg/TableMetadataParser.java index 13633a17f916..e1b99bd3d4e2 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadataParser.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadataParser.java @@ -110,8 +110,6 @@ private TableMetadataParser() {} static final String METADATA_LOG = "metadata-log"; static final String STATISTICS = "statistics"; static final String PARTITION_STATISTICS = "partition-statistics"; - static final String ROW_LINEAGE = "row-lineage"; - static final String NEXT_ROW_ID = "next-row-id"; public static void overwrite(TableMetadata metadata, OutputFile outputFile) { internalWrite(metadata, outputFile, true); diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 10b8e6915d4f..093239ac0725 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -169,12 +169,9 @@ public void testJsonConversion(int formatVersion) throws Exception { TableMetadata expected = MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .setMetadataLocation(null) .setFormatVersion(formatVersion) - .setUuid(UUID.randomUUID().toString()) .setLocation(TEST_LOCATION) .setLastSequenceNumber(SEQ_NO) - .setLastUpdatedMillis(System.currentTimeMillis()) .setLastColumnId(3) .setCurrentSchemaId(7) .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) @@ -186,13 +183,10 @@ public void testJsonConversion(int formatVersion) throws Exception { .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .setSnapshotsSupplier(null) .setSnapshotLog(snapshotLog) - .setMetadataHistory(ImmutableList.of()) .setRefs(refs) .setStatisticsFiles(statisticsFiles) .setPartitionStatisticsFiles(partitionStatisticsFiles) - .setChanges(ImmutableList.of()) .build(); String asJson = TableMetadataParser.toJson(expected); @@ -259,7 +253,6 @@ public void testBackwardCompat() throws Exception { TableMetadata expected = MetadataTestUtils.buildTestTableMetadataFromEmpty(1) - .setMetadataLocation(null) .setFormatVersion(1) .setUuid(null) .setLocation(TEST_LOCATION) @@ -276,13 +269,6 @@ public void testBackwardCompat() throws Exception { .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .setSnapshotsSupplier(null) - .setSnapshotLog(ImmutableList.of()) - .setMetadataHistory(ImmutableList.of()) - .setRefs(ImmutableMap.of()) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build(); String asJson = toJsonWithoutSpecAndSchemaList(expected); @@ -371,12 +357,9 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { assertThatThrownBy( () -> MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .setMetadataLocation(null) .setFormatVersion(formatVersion) - .setUuid(UUID.randomUUID().toString()) .setLocation(TEST_LOCATION) .setLastSequenceNumber(SEQ_NO) - .setLastUpdatedMillis(System.currentTimeMillis()) .setLastColumnId(3) .setCurrentSchemaId(7) .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) @@ -388,13 +371,8 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .setSnapshotsSupplier(null) .setSnapshotLog(snapshotLog) - .setMetadataHistory(ImmutableList.of()) .setRefs(refs) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Current snapshot ID does not match main branch"); @@ -420,12 +398,9 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { assertThatThrownBy( () -> MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .setMetadataLocation(null) .setFormatVersion(formatVersion) - .setUuid(UUID.randomUUID().toString()) .setLocation(TEST_LOCATION) .setLastSequenceNumber(SEQ_NO) - .setLastUpdatedMillis(System.currentTimeMillis()) .setLastColumnId(3) .setCurrentSchemaId(7) .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) @@ -435,14 +410,9 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { .setDefaultSortOrderId(3) .setSortOrders(ImmutableList.of(SORT_ORDER_3)) .setProperties(ImmutableMap.of("property", "value")) - .setCurrentSnapshotId(-1) .setSnapshots(ImmutableList.of(snapshot)) .setSnapshotsSupplier(null) - .setMetadataHistory(ImmutableList.of()) .setRefs(refs) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("Current snapshot is not set, but main branch exists"); @@ -463,12 +433,9 @@ public void testBranchSnapshotMissing(int formatVersion) { assertThatThrownBy( () -> MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .setMetadataLocation(null) .setFormatVersion(formatVersion) - .setUuid(UUID.randomUUID().toString()) .setLocation(TEST_LOCATION) .setLastSequenceNumber(SEQ_NO) - .setLastUpdatedMillis(System.currentTimeMillis()) .setLastColumnId(3) .setCurrentSchemaId(7) .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) @@ -478,14 +445,7 @@ public void testBranchSnapshotMissing(int formatVersion) { .setDefaultSortOrderId(3) .setSortOrders(ImmutableList.of(SORT_ORDER_3)) .setProperties(ImmutableMap.of("property", "value")) - .setCurrentSnapshotId(-1) - .setSnapshots(ImmutableList.of()) - .setSnapshotsSupplier(null) - .setMetadataHistory(ImmutableList.of()) .setRefs(refs) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageEndingWith("does not exist in the existing snapshots list"); @@ -568,7 +528,6 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception TableMetadata base = MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .setMetadataLocation(null) .setFormatVersion(formatVersion) .setUuid(UUID.randomUUID().toString()) .setLocation(TEST_LOCATION) @@ -588,10 +547,6 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception .setSnapshotsSupplier(null) .setSnapshotLog(reversedSnapshotLog) .setMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) - .setRefs(ImmutableMap.of()) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build(); String asJson = TableMetadataParser.toJson(base); @@ -648,7 +603,6 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) .setMetadataLocation(latestPreviousMetadata.file()) .setFormatVersion(formatVersion) - .setUuid(UUID.randomUUID().toString()) .setLocation(TEST_LOCATION) .setLastSequenceNumber(0) .setLastUpdatedMillis(currentTimestamp - 80) @@ -663,13 +617,8 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .setSnapshotsSupplier(null) .setSnapshotLog(reversedSnapshotLog) .setMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) - .setRefs(ImmutableMap.of()) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build(); previousMetadataLog.add(latestPreviousMetadata); @@ -741,7 +690,6 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) .setMetadataLocation(latestPreviousMetadata.file()) .setFormatVersion(formatVersion) - .setUuid(UUID.randomUUID().toString()) .setLocation(TEST_LOCATION) .setLastSequenceNumber(0) .setLastUpdatedMillis(currentTimestamp - 50) @@ -756,13 +704,8 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .setSnapshotsSupplier(null) .setSnapshotLog(reversedSnapshotLog) .setMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) - .setRefs(ImmutableMap.of()) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build(); previousMetadataLog.add(latestPreviousMetadata); @@ -838,7 +781,6 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) .setMetadataLocation(latestPreviousMetadata.file()) .setFormatVersion(formatVersion) - .setUuid(UUID.randomUUID().toString()) .setLocation(TEST_LOCATION) .setLastSequenceNumber(0) .setLastUpdatedMillis(currentTimestamp - 50) @@ -853,13 +795,8 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .setSnapshotsSupplier(null) .setSnapshotLog(reversedSnapshotLog) .setMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) - .setRefs(ImmutableMap.of()) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build(); previousMetadataLog.add(latestPreviousMetadata); @@ -886,7 +823,6 @@ public void testV2UUIDValidation(int formatVersion) { assertThatThrownBy( () -> MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) - .setMetadataLocation(null) .setFormatVersion(formatVersion) .setUuid(null) .setLocation(TEST_LOCATION) @@ -900,16 +836,6 @@ public void testV2UUIDValidation(int formatVersion) { .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) .setDefaultSortOrderId(3) .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .setProperties(ImmutableMap.of()) - .setCurrentSnapshotId(-1L) - .setSnapshots(ImmutableList.of()) - .setSnapshotsSupplier(null) - .setSnapshotLog(ImmutableList.of()) - .setMetadataHistory(ImmutableList.of()) - .setRefs(ImmutableMap.of()) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessage(String.format("UUID is required in format v%s", formatVersion)); @@ -922,7 +848,6 @@ public void testVersionValidation() { assertThatThrownBy( () -> MetadataTestUtils.buildTestTableMetadataFromEmpty(unsupportedVersion) - .setMetadataLocation(null) .setFormatVersion(unsupportedVersion) .setUuid(null) .setLocation(TEST_LOCATION) @@ -936,16 +861,6 @@ public void testVersionValidation() { .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) .setDefaultSortOrderId(3) .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .setProperties(ImmutableMap.of()) - .setCurrentSnapshotId(-1L) - .setSnapshots(ImmutableList.of()) - .setSnapshotsSupplier(null) - .setSnapshotLog(ImmutableList.of()) - .setMetadataHistory(ImmutableList.of()) - .setRefs(ImmutableMap.of()) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessage( @@ -969,12 +884,9 @@ public void testVersionValidation() { // should be allowed in the supported version assertThat( MetadataTestUtils.buildTestTableMetadataFromEmpty(supportedVersion) - .setMetadataLocation(null) .setFormatVersion(supportedVersion) - .setUuid(UUID.randomUUID().toString()) .setLocation(TEST_LOCATION) .setLastSequenceNumber(SEQ_NO) - .setLastUpdatedMillis(System.currentTimeMillis()) .setLastColumnId(LAST_ASSIGNED_COLUMN_ID) .setCurrentSchemaId(7) .setSchemas(ImmutableList.of(TEST_SCHEMA)) @@ -983,16 +895,6 @@ public void testVersionValidation() { .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) .setDefaultSortOrderId(3) .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .setProperties(ImmutableMap.of()) - .setCurrentSnapshotId(-1L) - .setSnapshots(ImmutableList.of()) - .setSnapshotsSupplier(null) - .setSnapshotLog(ImmutableList.of()) - .setMetadataHistory(ImmutableList.of()) - .setRefs(ImmutableMap.of()) - .setStatisticsFiles(ImmutableList.of()) - .setPartitionStatisticsFiles(ImmutableList.of()) - .setChanges(ImmutableList.of()) .build()) .isNotNull(); From 4aa3158672d7fc5e7ac02a139307fd6c2ecad543 Mon Sep 17 00:00:00 2001 From: Honah J Date: Fri, 10 Jan 2025 22:10:23 -0800 Subject: [PATCH 11/15] Add BaseSnapshotBuilder --- .../org/apache/iceberg/MetadataTestUtils.java | 97 ++++++- .../apache/iceberg/TestDataTaskParser.java | 24 +- .../iceberg/TestMetadataUpdateParser.java | 36 +-- .../org/apache/iceberg/TestSnapshotJson.java | 59 ++-- .../org/apache/iceberg/TestTableMetadata.java | 253 +++++++++++------- 5 files changed, 334 insertions(+), 135 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java b/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java index f3bb86616201..e8e2b2edcd6f 100644 --- a/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java +++ b/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.UUID; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -33,7 +34,7 @@ public class MetadataTestUtils { private MetadataTestUtils() {} - public static TableMetadataBuilder buildTestTableMetadataFromEmpty(int formatVersion) { + public static TableMetadataBuilder buildTestTableMetadata(int formatVersion) { return new TableMetadataBuilder(formatVersion); } @@ -238,4 +239,98 @@ public TableMetadata build() { ImmutableList.copyOf(changes)); } } + + public static BaseSnapshotBuilder buildTestSnapshot() { + return new BaseSnapshotBuilder(); + } + + public static class BaseSnapshotBuilder { + private long snapshotId; + private Long parentId; + private long sequenceNumber; + private long timestampMillis; + private String manifestListLocation; + private String operation; + private Map summary; + private Integer schemaId; + private String[] v1ManifestLocations; + + private BaseSnapshotBuilder() { + this.snapshotId = -1L; + this.sequenceNumber = -1L; + this.timestampMillis = System.currentTimeMillis(); + this.summary = ImmutableMap.of(); + } + + public BaseSnapshotBuilder setSnapshotId(long snapshotId) { + this.snapshotId = snapshotId; + return this; + } + + public BaseSnapshotBuilder setParentId(Long parentId) { + this.parentId = parentId; + return this; + } + + public BaseSnapshotBuilder setSequenceNumber(long sequenceNumber) { + this.sequenceNumber = sequenceNumber; + return this; + } + + public BaseSnapshotBuilder setTimestampMillis(long timestampMillis) { + this.timestampMillis = timestampMillis; + return this; + } + + public BaseSnapshotBuilder setManifestListLocation(String manifestListLocation) { + this.manifestListLocation = manifestListLocation; + return this; + } + + public BaseSnapshotBuilder setOperation(String operation) { + this.operation = operation; + return this; + } + + public BaseSnapshotBuilder setSummary(Map summary) { + this.summary = summary; + return this; + } + + public BaseSnapshotBuilder setSchemaId(Integer schemaId) { + this.schemaId = schemaId; + return this; + } + + public BaseSnapshotBuilder setV1ManifestLocations(String[] v1ManifestLocations) { + this.v1ManifestLocations = v1ManifestLocations; + return this; + } + + public Snapshot build() { + Preconditions.checkArgument( + manifestListLocation != null || v1ManifestLocations != null, + "Cannot set both ManifestListLocation and V1ManifestLocations"); + if (v1ManifestLocations != null) { + return new BaseSnapshot( + sequenceNumber, + snapshotId, + parentId, + timestampMillis, + operation, + summary, + schemaId, + v1ManifestLocations); + } + return new BaseSnapshot( + sequenceNumber, + snapshotId, + parentId, + timestampMillis, + operation, + summary, + schemaId, + manifestListLocation); + } + } } diff --git a/core/src/test/java/org/apache/iceberg/TestDataTaskParser.java b/core/src/test/java/org/apache/iceberg/TestDataTaskParser.java index 5a3d119046f5..ae1b126524f4 100644 --- a/core/src/test/java/org/apache/iceberg/TestDataTaskParser.java +++ b/core/src/test/java/org/apache/iceberg/TestDataTaskParser.java @@ -193,10 +193,26 @@ private DataTask createDataTask() { List snapshots = Arrays.asList( - new BaseSnapshot( - 1L, 1L, null, 1234567890000L, "append", summary1, 1, "file:/tmp/manifest1.avro"), - new BaseSnapshot( - 2L, 2L, 1L, 9876543210000L, "append", summary2, 1, "file:/tmp/manifest2.avro")); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(1L) + .setSnapshotId(1L) + .setParentId(null) + .setTimestampMillis(1234567890000L) + .setOperation("append") + .setSummary(summary1) + .setSchemaId(1) + .setManifestListLocation("file:/tmp/manifest1.avro") + .build(), + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(2L) + .setSnapshotId(2L) + .setParentId(1L) + .setTimestampMillis(9876543210000L) + .setOperation("append") + .setSummary(summary2) + .setSchemaId(1) + .setManifestListLocation("file:/tmp/manifest2.avro") + .build()); return StaticDataTask.of( Files.localInput("file:/tmp/metadata2.json"), diff --git a/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java b/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java index 6a65bf762880..cac4eb0ddc3a 100644 --- a/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java +++ b/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java @@ -364,15 +364,15 @@ public void testAddSnapshotToJson() throws IOException { String manifestList = createManifestListWithManifestFiles(snapshotId, parentId); Snapshot snapshot = - new BaseSnapshot( - 0, - snapshotId, - parentId, - System.currentTimeMillis(), - DataOperations.REPLACE, - ImmutableMap.of("files-added", "4", "files-deleted", "100"), - schemaId, - manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(snapshotId) + .setParentId(parentId) + .setOperation(DataOperations.REPLACE) + .setSummary(ImmutableMap.of("files-added", "4", "files-deleted", "100")) + .setSchemaId(schemaId) + .setManifestListLocation(manifestList) + .build(); String snapshotJson = SnapshotParser.toJson(snapshot, /* pretty */ false); String expected = String.format("{\"action\":\"%s\",\"snapshot\":%s}", action, snapshotJson); MetadataUpdate update = new MetadataUpdate.AddSnapshot(snapshot); @@ -392,15 +392,15 @@ public void testAddSnapshotFromJson() throws IOException { String manifestList = createManifestListWithManifestFiles(snapshotId, parentId); Snapshot snapshot = - new BaseSnapshot( - 0, - snapshotId, - parentId, - System.currentTimeMillis(), - DataOperations.REPLACE, - summary, - schemaId, - manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(snapshotId) + .setParentId(parentId) + .setOperation(DataOperations.REPLACE) + .setSummary(summary) + .setSchemaId(schemaId) + .setManifestListLocation(manifestList) + .build(); String snapshotJson = SnapshotParser.toJson(snapshot, /* pretty */ false); String json = String.format("{\"action\":\"%s\",\"snapshot\":%s}", action, snapshotJson); MetadataUpdate expected = new MetadataUpdate.AddSnapshot(snapshot); diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java b/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java index e4c2ba5ec2df..d27b11f436a6 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java @@ -42,8 +42,15 @@ public void testJsonConversion() throws IOException { String manifestList = createManifestListWithManifestFiles(snapshotId, parentId); Snapshot expected = - new BaseSnapshot( - 0, snapshotId, parentId, System.currentTimeMillis(), null, null, 1, manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(snapshotId) + .setParentId(parentId) + .setOperation(null) + .setSummary(null) + .setSchemaId(1) + .setManifestListLocation(manifestList) + .build(); String json = SnapshotParser.toJson(expected); Snapshot snapshot = SnapshotParser.fromJson(json); @@ -61,8 +68,15 @@ public void testJsonConversionWithoutSchemaId() throws IOException { String manifestList = createManifestListWithManifestFiles(snapshotId, parentId); Snapshot expected = - new BaseSnapshot( - 0, snapshotId, parentId, System.currentTimeMillis(), null, null, null, manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(snapshotId) + .setParentId(parentId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); String json = SnapshotParser.toJson(expected); Snapshot snapshot = SnapshotParser.fromJson(json); @@ -81,15 +95,15 @@ public void testJsonConversionWithOperation() throws IOException { String manifestList = createManifestListWithManifestFiles(id, parentId); Snapshot expected = - new BaseSnapshot( - 0, - id, - parentId, - System.currentTimeMillis(), - DataOperations.REPLACE, - ImmutableMap.of("files-added", "4", "files-deleted", "100"), - 3, - manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(id) + .setParentId(parentId) + .setOperation(DataOperations.REPLACE) + .setSummary(ImmutableMap.of("files-added", "4", "files-deleted", "100")) + .setSchemaId(3) + .setManifestListLocation(manifestList) + .build(); String json = SnapshotParser.toJson(expected); Snapshot snapshot = SnapshotParser.fromJson(json); @@ -115,15 +129,16 @@ public void testJsonConversionWithV1Manifests() { // this creates a V1 snapshot with manifests long timestampMillis = System.currentTimeMillis(); Snapshot expected = - new BaseSnapshot( - 0, - id, - parentId, - timestampMillis, - DataOperations.REPLACE, - ImmutableMap.of("files-added", "4", "files-deleted", "100"), - 3, - new String[] {"/tmp/manifest1.avro", "/tmp/manifest2.avro"}); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0) + .setSnapshotId(id) + .setParentId(parentId) + .setTimestampMillis(timestampMillis) + .setOperation(DataOperations.REPLACE) + .setSummary(ImmutableMap.of("files-added", "4", "files-deleted", "100")) + .setSchemaId(3) + .setV1ManifestLocations(new String[] {"/tmp/manifest1.avro", "/tmp/manifest2.avro"}) + .build(); String expectedJson = String.format( diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 093239ac0725..a1e201c6b0e8 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -112,23 +112,32 @@ public void testJsonConversion(int formatVersion) throws Exception { String manifestList = createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = - new BaseSnapshot( - 0, previousSnapshotId, null, previousSnapshotId, null, null, null, manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(previousSnapshotId) + .setParentId(null) + .setTimestampMillis(previousSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); long currentSnapshotId = System.currentTimeMillis(); manifestList = createManifestListWithManifestFile( currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = - new BaseSnapshot( - 0, - currentSnapshotId, - previousSnapshotId, - currentSnapshotId, - null, - null, - 7, - manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(currentSnapshotId) + .setParentId(previousSnapshotId) + .setTimestampMillis(currentSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(7) + .setManifestListLocation(manifestList) + .build(); List snapshotLog = ImmutableList.builder() @@ -168,7 +177,7 @@ public void testJsonConversion(int formatVersion) throws Exception { .build()); TableMetadata expected = - MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + MetadataTestUtils.buildTestTableMetadata(formatVersion) .setFormatVersion(formatVersion) .setLocation(TEST_LOCATION) .setLastSequenceNumber(SEQ_NO) @@ -233,26 +242,35 @@ public void testBackwardCompat() throws Exception { String manifestList = createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = - new BaseSnapshot( - 0, previousSnapshotId, null, previousSnapshotId, null, null, null, manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(previousSnapshotId) + .setParentId(null) + .setTimestampMillis(previousSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); long currentSnapshotId = System.currentTimeMillis(); manifestList = createManifestListWithManifestFile( currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = - new BaseSnapshot( - 0, - currentSnapshotId, - previousSnapshotId, - currentSnapshotId, - null, - null, - null, - manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(currentSnapshotId) + .setParentId(previousSnapshotId) + .setTimestampMillis(currentSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); TableMetadata expected = - MetadataTestUtils.buildTestTableMetadataFromEmpty(1) + MetadataTestUtils.buildTestTableMetadata(1) .setFormatVersion(1) .setUuid(null) .setLocation(TEST_LOCATION) @@ -320,8 +338,16 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { String manifestList = createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = - new BaseSnapshot( - 0, previousSnapshotId, null, previousSnapshotId, null, null, null, manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(previousSnapshotId) + .setParentId(null) + .setTimestampMillis(previousSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); long currentSnapshotId = System.currentTimeMillis(); manifestList = @@ -329,15 +355,16 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = - new BaseSnapshot( - 0, - currentSnapshotId, - previousSnapshotId, - currentSnapshotId, - null, - null, - 7, - manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(currentSnapshotId) + .setParentId(previousSnapshotId) + .setTimestampMillis(currentSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(7) + .setManifestListLocation(manifestList) + .build(); List snapshotLog = ImmutableList.builder() @@ -356,7 +383,7 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { assertThatThrownBy( () -> - MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + MetadataTestUtils.buildTestTableMetadata(formatVersion) .setFormatVersion(formatVersion) .setLocation(TEST_LOCATION) .setLastSequenceNumber(SEQ_NO) @@ -388,7 +415,16 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { String manifestList = createManifestListWithManifestFile(snapshotId, null, "file:/tmp/manifest1.avro"); Snapshot snapshot = - new BaseSnapshot(0, snapshotId, null, snapshotId, null, null, null, manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(snapshotId) + .setParentId(null) + .setTimestampMillis(snapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); Schema schema = new Schema(6, Types.NestedField.required(10, "x", Types.StringType.get())); @@ -397,7 +433,7 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { assertThatThrownBy( () -> - MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + MetadataTestUtils.buildTestTableMetadata(formatVersion) .setFormatVersion(formatVersion) .setLocation(TEST_LOCATION) .setLastSequenceNumber(SEQ_NO) @@ -432,7 +468,7 @@ public void testBranchSnapshotMissing(int formatVersion) { assertThatThrownBy( () -> - MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + MetadataTestUtils.buildTestTableMetadata(formatVersion) .setFormatVersion(formatVersion) .setLocation(TEST_LOCATION) .setLastSequenceNumber(SEQ_NO) @@ -501,23 +537,32 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception String manifestList = createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = - new BaseSnapshot( - 0, previousSnapshotId, null, previousSnapshotId, null, null, null, manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(previousSnapshotId) + .setParentId(null) + .setTimestampMillis(previousSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); long currentSnapshotId = System.currentTimeMillis(); manifestList = createManifestListWithManifestFile( currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = - new BaseSnapshot( - 0, - currentSnapshotId, - previousSnapshotId, - currentSnapshotId, - null, - null, - null, - manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(currentSnapshotId) + .setParentId(previousSnapshotId) + .setTimestampMillis(currentSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); List reversedSnapshotLog = Lists.newArrayList(); long currentTimestamp = System.currentTimeMillis(); @@ -527,7 +572,7 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception currentTimestamp, "/tmp/000001-" + UUID.randomUUID() + ".metadata.json")); TableMetadata base = - MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + MetadataTestUtils.buildTestTableMetadata(formatVersion) .setFormatVersion(formatVersion) .setUuid(UUID.randomUUID().toString()) .setLocation(TEST_LOCATION) @@ -563,23 +608,33 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept String manifestList = createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = - new BaseSnapshot( - 0, previousSnapshotId, null, previousSnapshotId, null, null, null, manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(previousSnapshotId) + .setParentId(null) + .setTimestampMillis(previousSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); + long currentSnapshotId = System.currentTimeMillis(); manifestList = createManifestListWithManifestFile( currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = - new BaseSnapshot( - 0, - currentSnapshotId, - previousSnapshotId, - currentSnapshotId, - null, - null, - null, - manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(currentSnapshotId) + .setParentId(previousSnapshotId) + .setTimestampMillis(currentSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); List reversedSnapshotLog = Lists.newArrayList(); reversedSnapshotLog.add( @@ -600,7 +655,7 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept currentTimestamp - 80, "/tmp/000003-" + UUID.randomUUID() + ".metadata.json"); TableMetadata base = - MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + MetadataTestUtils.buildTestTableMetadata(formatVersion) .setMetadataLocation(latestPreviousMetadata.file()) .setFormatVersion(formatVersion) .setLocation(TEST_LOCATION) @@ -641,23 +696,32 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti String manifestList = createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = - new BaseSnapshot( - 0, previousSnapshotId, null, previousSnapshotId, null, null, null, manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(previousSnapshotId) + .setParentId(null) + .setTimestampMillis(previousSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); long currentSnapshotId = System.currentTimeMillis(); manifestList = createManifestListWithManifestFile( currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = - new BaseSnapshot( - 0, - currentSnapshotId, - previousSnapshotId, - currentSnapshotId, - null, - null, - null, - manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(currentSnapshotId) + .setParentId(previousSnapshotId) + .setTimestampMillis(currentSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); List reversedSnapshotLog = Lists.newArrayList(); reversedSnapshotLog.add( @@ -687,7 +751,7 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti currentTimestamp - 50, "/tmp/000006-" + UUID.randomUUID() + ".metadata.json"); TableMetadata base = - MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + MetadataTestUtils.buildTestTableMetadata(formatVersion) .setMetadataLocation(latestPreviousMetadata.file()) .setFormatVersion(formatVersion) .setLocation(TEST_LOCATION) @@ -732,23 +796,32 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx String manifestList = createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = - new BaseSnapshot( - 0, previousSnapshotId, null, previousSnapshotId, null, null, null, manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(previousSnapshotId) + .setParentId(null) + .setTimestampMillis(previousSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); long currentSnapshotId = System.currentTimeMillis(); manifestList = createManifestListWithManifestFile( currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = - new BaseSnapshot( - 0, - currentSnapshotId, - previousSnapshotId, - currentSnapshotId, - null, - null, - null, - manifestList); + MetadataTestUtils.buildTestSnapshot() + .setSequenceNumber(0L) + .setSnapshotId(currentSnapshotId) + .setParentId(previousSnapshotId) + .setTimestampMillis(currentSnapshotId) + .setOperation(null) + .setSummary(null) + .setSchemaId(null) + .setManifestListLocation(manifestList) + .build(); List reversedSnapshotLog = Lists.newArrayList(); reversedSnapshotLog.add( @@ -778,7 +851,7 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx currentTimestamp - 50, "/tmp/000006-" + UUID.randomUUID() + ".metadata.json"); TableMetadata base = - MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + MetadataTestUtils.buildTestTableMetadata(formatVersion) .setMetadataLocation(latestPreviousMetadata.file()) .setFormatVersion(formatVersion) .setLocation(TEST_LOCATION) @@ -822,7 +895,7 @@ public void testV2UUIDValidation(int formatVersion) { assertThatThrownBy( () -> - MetadataTestUtils.buildTestTableMetadataFromEmpty(formatVersion) + MetadataTestUtils.buildTestTableMetadata(formatVersion) .setFormatVersion(formatVersion) .setUuid(null) .setLocation(TEST_LOCATION) @@ -847,7 +920,7 @@ public void testVersionValidation() { int unsupportedVersion = supportedVersion + 1; assertThatThrownBy( () -> - MetadataTestUtils.buildTestTableMetadataFromEmpty(unsupportedVersion) + MetadataTestUtils.buildTestTableMetadata(unsupportedVersion) .setFormatVersion(unsupportedVersion) .setUuid(null) .setLocation(TEST_LOCATION) @@ -883,7 +956,7 @@ public void testVersionValidation() { // should be allowed in the supported version assertThat( - MetadataTestUtils.buildTestTableMetadataFromEmpty(supportedVersion) + MetadataTestUtils.buildTestTableMetadata(supportedVersion) .setFormatVersion(supportedVersion) .setLocation(TEST_LOCATION) .setLastSequenceNumber(SEQ_NO) From 6e9b00e0ffaca9250d6d8cbef727990569c6de47 Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 14 Jan 2025 20:10:02 -0800 Subject: [PATCH 12/15] switch to FieldSource in TestHelpers --- .../org/apache/iceberg/TestTableMetadata.java | 82 +++++++++---------- 1 file changed, 39 insertions(+), 43 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index a1e201c6b0e8..4897211b43b0 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -69,6 +69,7 @@ import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.FieldSource; import org.junit.jupiter.params.provider.MethodSource; public class TestTableMetadata { @@ -97,17 +98,12 @@ public class TestTableMetadata { public TableOperations ops = new LocalTableOperations(temp); - private static Stream formatVersionsProvider() { - return Stream.of(1, 2, 3); - } - @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") @SuppressWarnings("MethodLength") public void testJsonConversion(int formatVersion) throws Exception { - assumeThat(formatVersion).isGreaterThanOrEqualTo(2); - long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); + long lastSequenceNumber = formatVersion >= 2 ? SEQ_NO : 0L; String manifestList = createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); @@ -180,7 +176,7 @@ public void testJsonConversion(int formatVersion) throws Exception { MetadataTestUtils.buildTestTableMetadata(formatVersion) .setFormatVersion(formatVersion) .setLocation(TEST_LOCATION) - .setLastSequenceNumber(SEQ_NO) + .setLastSequenceNumber(lastSequenceNumber) .setLastColumnId(3) .setCurrentSchemaId(7) .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) @@ -329,7 +325,7 @@ public void testBackwardCompat() throws Exception { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testInvalidMainBranch(int formatVersion) throws IOException { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -406,7 +402,7 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testMainWithoutCurrent(int formatVersion) throws IOException { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -455,7 +451,7 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testBranchSnapshotMissing(int formatVersion) { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -530,7 +526,7 @@ private static String toJsonWithoutSpecAndSchemaList(TableMetadata metadata) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); @@ -601,7 +597,7 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); @@ -689,7 +685,7 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); @@ -789,7 +785,7 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); @@ -889,7 +885,7 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testV2UUIDValidation(int formatVersion) { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -983,7 +979,7 @@ public void testVersionValidation() { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testParserVersionValidation(int formatVersion) throws Exception { String supportedVersion = readTableMetadataInputFile(String.format("TableMetadataV%sValid.json", formatVersion)); @@ -1000,7 +996,7 @@ public void testParserUnsupportedVersion() throws Exception { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testParserV2PartitionSpecsValidation(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1013,7 +1009,7 @@ public void testParserV2PartitionSpecsValidation(int formatVersion) throws Excep } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testParserLastAssignedFieldIdValidation(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1026,7 +1022,7 @@ public void testParserLastAssignedFieldIdValidation(int formatVersion) throws Ex } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testParserSortOrderValidation(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1039,7 +1035,7 @@ public void testParserSortOrderValidation(int formatVersion) throws Exception { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testParserCurrentSchemaIdValidation(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1052,7 +1048,7 @@ public void testParserCurrentSchemaIdValidation(int formatVersion) throws Except } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testParserV2SchemasValidation(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1070,7 +1066,7 @@ private String readTableMetadataInputFile(String fileName) throws Exception { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testNewTableMetadataReassignmentAllIds(int formatVersion) throws Exception { Schema schema = new Schema( @@ -1162,7 +1158,7 @@ public void testBuildReplacementForV1Table() { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testBuildReplacementForV2AndV3Table(int formatVersion) { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1200,7 +1196,7 @@ public void testBuildReplacementForV2AndV3Table(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testSortOrder(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1218,7 +1214,7 @@ public void testSortOrder(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testUpdateSortOrder(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1263,7 +1259,7 @@ public void testUpdateSortOrder(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1279,7 +1275,7 @@ public void testStatistics(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testSetStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1324,7 +1320,7 @@ public void testSetStatistics(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testRemoveStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1362,7 +1358,7 @@ public void testRemoveStatistics(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testPartitionStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1380,7 +1376,7 @@ public void testPartitionStatistics(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testSetPartitionStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1435,7 +1431,7 @@ public void testSetPartitionStatistics(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testRemovePartitionStatistics(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1481,7 +1477,7 @@ public void testRemovePartitionStatistics(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testParseSchemaIdentifierFields(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1493,7 +1489,7 @@ public void testParseSchemaIdentifierFields(int formatVersion) throws Exception } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testParseMinimal(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1508,7 +1504,7 @@ public void testParseMinimal(int formatVersion) throws Exception { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testUpdateSchemaIdentifierFields(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1531,7 +1527,7 @@ public void testUpdateSchemaIdentifierFields(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testUpdateSchema(int formatVersion) { Schema schema = new Schema(0, Types.NestedField.required(1, "y", Types.LongType.get(), "comment")); @@ -1603,7 +1599,7 @@ schema, new Schema(1, schema2.columns()), new Schema(2, schema3.columns())), } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testCreateMetadataThroughTableProperty(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1688,7 +1684,7 @@ public void testUpgradeMetadataThroughTableProperty(int baseFormatVersion, int n } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testParseStatisticsFiles(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1712,7 +1708,7 @@ public void testParseStatisticsFiles(int formatVersion) throws Exception { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testParsePartitionStatisticsFiles(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1732,7 +1728,7 @@ public void testParsePartitionStatisticsFiles(int formatVersion) throws Exceptio } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testNoReservedPropertyForTableMetadataCreation(int formatVersion) { Schema schema = new Schema(Types.NestedField.required(10, "x", Types.StringType.get())); @@ -1765,7 +1761,7 @@ public void testNoReservedPropertyForTableMetadataCreation(int formatVersion) { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testNoTrailingLocationSlash(int formatVersion) { String locationWithSlash = "/with_trailing_slash/"; String locationWithoutSlash = "/with_trailing_slash"; @@ -1798,7 +1794,7 @@ private String createManifestListWithManifestFile( } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void buildReplacementKeepsSnapshotLog(int formatVersion) throws Exception { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); @@ -1843,7 +1839,7 @@ public void removeRefKeepsSnapshotLog() throws Exception { } @ParameterizedTest - @MethodSource("formatVersionsProvider") + @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void onlyMetadataLocationIsUpdatedWithoutTimestampAndMetadataLogEntry(int formatVersion) { String uuid = "386b9f01-002b-4d8c-b77f-42c3fd3b7c9b"; TableMetadata metadata = From d22a24bb0455b7e173cf65ced9b354f37a688cbf Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 14 Jan 2025 21:49:45 -0800 Subject: [PATCH 13/15] Add defaults for snapshot test builder --- .../org/apache/iceberg/MetadataTestUtils.java | 55 +++++++++++ .../iceberg/TestMetadataUpdateParser.java | 61 +++--------- .../org/apache/iceberg/TestSnapshotJson.java | 71 ++++--------- .../org/apache/iceberg/TestTableMetadata.java | 99 +++---------------- 4 files changed, 100 insertions(+), 186 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java b/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java index e8e2b2edcd6f..acc22afb424f 100644 --- a/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java +++ b/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java @@ -18,11 +18,16 @@ */ package org.apache.iceberg; +import static org.apache.iceberg.Files.localInput; import static org.apache.iceberg.TableMetadata.INITIAL_SEQUENCE_NUMBER; +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.stream.Collectors; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; @@ -32,6 +37,15 @@ public class MetadataTestUtils { + private static final long EXAMPLE_SNAPSHOT_ID = 3; + private static final long EXAMPLE_PARENT_ID = 1; + private static final Integer EXAMPLE_SCHEMA_ID = 2; + private static final long EXAMPLE_SEQUENCE_NUMBER = 0; + private static final int EXAMPLE_SPEC_ID = 5; + + public static final String EXAMPLE_MANIFEST_PATH_1 = "file:/tmp/manifest1.avro"; + public static final String EXAMPLE_MANIFEST_PATH_2 = "file:/tmp/manifest2.avro"; + private MetadataTestUtils() {} public static TableMetadataBuilder buildTestTableMetadata(int formatVersion) { @@ -244,6 +258,17 @@ public static BaseSnapshotBuilder buildTestSnapshot() { return new BaseSnapshotBuilder(); } + public static BaseSnapshotBuilder buildTestSnapshotWithExampleValues() { + return new BaseSnapshotBuilder() + .setSequenceNumber(EXAMPLE_SEQUENCE_NUMBER) + .setSnapshotId(EXAMPLE_SNAPSHOT_ID) + .setParentId(EXAMPLE_PARENT_ID) + .setOperation(DataOperations.REPLACE) + .setSummary(ImmutableMap.of("files-added", "4", "files-deleted", "100")) + .setSchemaId(EXAMPLE_SCHEMA_ID) + .setTimestampMillis(System.currentTimeMillis()); + } + public static class BaseSnapshotBuilder { private long snapshotId; private Long parentId; @@ -332,5 +357,35 @@ public Snapshot build() { schemaId, manifestListLocation); } + + public Snapshot buildWithExampleManifestList(Path temp, List manifestFiles) + throws IOException { + Preconditions.checkArgument( + manifestListLocation == null && v1ManifestLocations == null, + "An example manifest list with manifest files will be created"); + + this.manifestListLocation = + createManifestListWithManifestFiles(snapshotId, parentId, temp, manifestFiles); + return build(); + } + } + + private static String createManifestListWithManifestFiles( + long snapshotId, Long parentSnapshotId, Path temp, List manifestFiles) + throws IOException { + File manifestList = File.createTempFile("manifests", null, temp.toFile()); + manifestList.deleteOnExit(); + + List manifests = + manifestFiles.stream() + .map(name -> new GenericManifestFile(localInput(name), EXAMPLE_SPEC_ID, snapshotId)) + .collect(Collectors.toList()); + + try (ManifestListWriter writer = + ManifestLists.write(1, Files.localOutput(manifestList), snapshotId, parentSnapshotId, 0)) { + writer.addAll(manifests); + } + + return localInput(manifestList).location(); } } diff --git a/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java b/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java index cac4eb0ddc3a..57de6ce2a47a 100644 --- a/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java +++ b/core/src/test/java/org/apache/iceberg/TestMetadataUpdateParser.java @@ -18,12 +18,10 @@ */ package org.apache.iceberg; -import static org.apache.iceberg.Files.localInput; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.fail; -import java.io.File; import java.io.IOException; import java.nio.file.Path; import java.util.List; @@ -357,22 +355,14 @@ public void testSetDefaultSortOrderFromJson() { @Test public void testAddSnapshotToJson() throws IOException { String action = MetadataUpdateParser.ADD_SNAPSHOT; - long parentId = 1; - long snapshotId = 2; - int schemaId = 3; - - String manifestList = createManifestListWithManifestFiles(snapshotId, parentId); Snapshot snapshot = - MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) - .setSnapshotId(snapshotId) - .setParentId(parentId) - .setOperation(DataOperations.REPLACE) - .setSummary(ImmutableMap.of("files-added", "4", "files-deleted", "100")) - .setSchemaId(schemaId) - .setManifestListLocation(manifestList) - .build(); + MetadataTestUtils.buildTestSnapshotWithExampleValues() + .buildWithExampleManifestList( + temp, + ImmutableList.of( + MetadataTestUtils.EXAMPLE_MANIFEST_PATH_1, + MetadataTestUtils.EXAMPLE_MANIFEST_PATH_2)); String snapshotJson = SnapshotParser.toJson(snapshot, /* pretty */ false); String expected = String.format("{\"action\":\"%s\",\"snapshot\":%s}", action, snapshotJson); MetadataUpdate update = new MetadataUpdate.AddSnapshot(snapshot); @@ -385,22 +375,13 @@ public void testAddSnapshotToJson() throws IOException { @Test public void testAddSnapshotFromJson() throws IOException { String action = MetadataUpdateParser.ADD_SNAPSHOT; - long parentId = 1; - long snapshotId = 2; - int schemaId = 3; - Map summary = ImmutableMap.of("files-added", "4", "files-deleted", "100"); - - String manifestList = createManifestListWithManifestFiles(snapshotId, parentId); Snapshot snapshot = - MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) - .setSnapshotId(snapshotId) - .setParentId(parentId) - .setOperation(DataOperations.REPLACE) - .setSummary(summary) - .setSchemaId(schemaId) - .setManifestListLocation(manifestList) - .build(); + MetadataTestUtils.buildTestSnapshotWithExampleValues() + .buildWithExampleManifestList( + temp, + ImmutableList.of( + MetadataTestUtils.EXAMPLE_MANIFEST_PATH_1, + MetadataTestUtils.EXAMPLE_MANIFEST_PATH_2)); String snapshotJson = SnapshotParser.toJson(snapshot, /* pretty */ false); String json = String.format("{\"action\":\"%s\",\"snapshot\":%s}", action, snapshotJson); MetadataUpdate expected = new MetadataUpdate.AddSnapshot(snapshot); @@ -1257,22 +1238,4 @@ private static void assertEqualsRemovePartitionSpecs( MetadataUpdate.RemovePartitionSpecs expected, MetadataUpdate.RemovePartitionSpecs actual) { assertThat(actual.specIds()).containsExactlyInAnyOrderElementsOf(expected.specIds()); } - - private String createManifestListWithManifestFiles(long snapshotId, Long parentSnapshotId) - throws IOException { - File manifestList = File.createTempFile("manifests", null, temp.toFile()); - manifestList.deleteOnExit(); - - List manifests = - ImmutableList.of( - new GenericManifestFile(localInput("file:/tmp/manifest1.avro"), 0, snapshotId), - new GenericManifestFile(localInput("file:/tmp/manifest2.avro"), 0, snapshotId)); - - try (ManifestListWriter writer = - ManifestLists.write(1, Files.localOutput(manifestList), snapshotId, parentSnapshotId, 0)) { - writer.addAll(manifests); - } - - return localInput(manifestList).location(); - } } diff --git a/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java b/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java index d27b11f436a6..3daffa5378c0 100644 --- a/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java +++ b/core/src/test/java/org/apache/iceberg/TestSnapshotJson.java @@ -18,13 +18,10 @@ */ package org.apache.iceberg; -import static org.apache.iceberg.Files.localInput; import static org.assertj.core.api.Assertions.assertThat; -import java.io.File; import java.io.IOException; import java.nio.file.Path; -import java.util.List; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.Test; @@ -37,20 +34,16 @@ public class TestSnapshotJson { @Test public void testJsonConversion() throws IOException { - int snapshotId = 23; - Long parentId = null; - String manifestList = createManifestListWithManifestFiles(snapshotId, parentId); - Snapshot expected = - MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) - .setSnapshotId(snapshotId) - .setParentId(parentId) + MetadataTestUtils.buildTestSnapshotWithExampleValues() .setOperation(null) .setSummary(null) .setSchemaId(1) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList( + temp, + ImmutableList.of( + MetadataTestUtils.EXAMPLE_MANIFEST_PATH_1, + MetadataTestUtils.EXAMPLE_MANIFEST_PATH_2)); String json = SnapshotParser.toJson(expected); Snapshot snapshot = SnapshotParser.fromJson(json); @@ -63,20 +56,16 @@ public void testJsonConversion() throws IOException { @Test public void testJsonConversionWithoutSchemaId() throws IOException { - int snapshotId = 23; - Long parentId = null; - String manifestList = createManifestListWithManifestFiles(snapshotId, parentId); - Snapshot expected = - MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) - .setSnapshotId(snapshotId) - .setParentId(parentId) + MetadataTestUtils.buildTestSnapshotWithExampleValues() .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList( + temp, + ImmutableList.of( + MetadataTestUtils.EXAMPLE_MANIFEST_PATH_1, + MetadataTestUtils.EXAMPLE_MANIFEST_PATH_2)); String json = SnapshotParser.toJson(expected); Snapshot snapshot = SnapshotParser.fromJson(json); @@ -89,21 +78,15 @@ public void testJsonConversionWithoutSchemaId() throws IOException { @Test public void testJsonConversionWithOperation() throws IOException { - long parentId = 1; - long id = 2; - - String manifestList = createManifestListWithManifestFiles(id, parentId); - Snapshot expected = - MetadataTestUtils.buildTestSnapshot() + MetadataTestUtils.buildTestSnapshotWithExampleValues() .setSequenceNumber(0L) - .setSnapshotId(id) - .setParentId(parentId) .setOperation(DataOperations.REPLACE) - .setSummary(ImmutableMap.of("files-added", "4", "files-deleted", "100")) - .setSchemaId(3) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList( + temp, + ImmutableList.of( + MetadataTestUtils.EXAMPLE_MANIFEST_PATH_1, + MetadataTestUtils.EXAMPLE_MANIFEST_PATH_2)); String json = SnapshotParser.toJson(expected); Snapshot snapshot = SnapshotParser.fromJson(json); @@ -174,24 +157,6 @@ public void testJsonConversionWithV1Manifests() { assertThat(snapshot.schemaId()).isEqualTo(expected.schemaId()); } - private String createManifestListWithManifestFiles(long snapshotId, Long parentSnapshotId) - throws IOException { - File manifestList = File.createTempFile("manifests", null, temp.toFile()); - manifestList.deleteOnExit(); - - List manifests = - ImmutableList.of( - new GenericManifestFile(localInput("file:/tmp/manifest1.avro"), 0, snapshotId), - new GenericManifestFile(localInput("file:/tmp/manifest2.avro"), 0, snapshotId)); - - try (ManifestListWriter writer = - ManifestLists.write(1, Files.localOutput(manifestList), snapshotId, parentSnapshotId, 0)) { - writer.addAll(manifests); - } - - return localInput(manifestList).location(); - } - @Test public void testJsonConversionSummaryWithoutOperation() { // This behavior is out of spec, but we don't want to fail on it. diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 4897211b43b0..9c99600cc845 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -18,7 +18,6 @@ */ package org.apache.iceberg; -import static org.apache.iceberg.Files.localInput; import static org.apache.iceberg.TableMetadataParser.CURRENT_SNAPSHOT_ID; import static org.apache.iceberg.TableMetadataParser.FORMAT_VERSION; import static org.apache.iceberg.TableMetadataParser.LAST_COLUMN_ID; @@ -36,7 +35,6 @@ import static org.junit.jupiter.params.provider.Arguments.arguments; import com.fasterxml.jackson.core.JsonGenerator; -import java.io.File; import java.io.IOException; import java.io.StringWriter; import java.io.UncheckedIOException; @@ -105,8 +103,6 @@ public void testJsonConversion(int formatVersion) throws Exception { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); long lastSequenceNumber = formatVersion >= 2 ? SEQ_NO : 0L; - String manifestList = - createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -116,13 +112,9 @@ public void testJsonConversion(int formatVersion) throws Exception { .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); - manifestList = - createManifestListWithManifestFile( - currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -132,8 +124,7 @@ public void testJsonConversion(int formatVersion) throws Exception { .setOperation(null) .setSummary(null) .setSchemaId(7) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); List snapshotLog = ImmutableList.builder() @@ -235,8 +226,6 @@ public void testBackwardCompat() throws Exception { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); - String manifestList = - createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -246,13 +235,9 @@ public void testBackwardCompat() throws Exception { .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); - manifestList = - createManifestListWithManifestFile( - currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -262,8 +247,7 @@ public void testBackwardCompat() throws Exception { .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); TableMetadata expected = MetadataTestUtils.buildTestTableMetadata(1) @@ -331,8 +315,6 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); - String manifestList = - createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -342,13 +324,9 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); - manifestList = - createManifestListWithManifestFile( - currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() @@ -359,8 +337,7 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { .setOperation(null) .setSummary(null) .setSchemaId(7) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); List snapshotLog = ImmutableList.builder() @@ -408,8 +385,6 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { long snapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); - String manifestList = - createManifestListWithManifestFile(snapshotId, null, "file:/tmp/manifest1.avro"); Snapshot snapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -419,8 +394,7 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); Schema schema = new Schema(6, Types.NestedField.required(10, "x", Types.StringType.get())); @@ -530,8 +504,6 @@ private static String toJsonWithoutSpecAndSchemaList(TableMetadata metadata) { public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); - String manifestList = - createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -541,13 +513,9 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); - manifestList = - createManifestListWithManifestFile( - currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -557,8 +525,7 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); List reversedSnapshotLog = Lists.newArrayList(); long currentTimestamp = System.currentTimeMillis(); @@ -601,8 +568,6 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); - String manifestList = - createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -612,14 +577,10 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); - manifestList = - createManifestListWithManifestFile( - currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -629,8 +590,7 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); List reversedSnapshotLog = Lists.newArrayList(); reversedSnapshotLog.add( @@ -689,8 +649,6 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); - String manifestList = - createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -700,13 +658,9 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); - manifestList = - createManifestListWithManifestFile( - currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -716,8 +670,7 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); List reversedSnapshotLog = Lists.newArrayList(); reversedSnapshotLog.add( @@ -789,8 +742,6 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); - String manifestList = - createManifestListWithManifestFile(previousSnapshotId, null, "file:/tmp/manifest1.avro"); Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -800,13 +751,9 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); - manifestList = - createManifestListWithManifestFile( - currentSnapshotId, previousSnapshotId, "file:/tmp/manifest2.avro"); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() .setSequenceNumber(0L) @@ -816,8 +763,7 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx .setOperation(null) .setSummary(null) .setSchemaId(null) - .setManifestListLocation(manifestList) - .build(); + .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); List reversedSnapshotLog = Lists.newArrayList(); reversedSnapshotLog.add( @@ -1778,21 +1724,6 @@ public void testNoTrailingLocationSlash(int formatVersion) { .isEqualTo(locationWithoutSlash); } - private String createManifestListWithManifestFile( - long snapshotId, Long parentSnapshotId, String manifestFile) throws IOException { - File manifestList = File.createTempFile("manifests", null, temp.toFile()); - manifestList.deleteOnExit(); - - try (ManifestListWriter writer = - ManifestLists.write(1, Files.localOutput(manifestList), snapshotId, parentSnapshotId, 0)) { - writer.addAll( - ImmutableList.of( - new GenericManifestFile(localInput(manifestFile), SPEC_5.specId(), snapshotId))); - } - - return localInput(manifestList).location(); - } - @ParameterizedTest @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void buildReplacementKeepsSnapshotLog(int formatVersion) throws Exception { From 1bc5075e8c0c66217c9b848ca0c972cf530e8a06 Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 14 Jan 2025 22:28:31 -0800 Subject: [PATCH 14/15] add defaults to Table metadata test builder --- .../org/apache/iceberg/MetadataTestUtils.java | 37 +++ .../org/apache/iceberg/TestTableMetadata.java | 261 ++---------------- 2 files changed, 54 insertions(+), 244 deletions(-) diff --git a/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java b/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java index acc22afb424f..506eb5e27e65 100644 --- a/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java +++ b/core/src/test/java/org/apache/iceberg/MetadataTestUtils.java @@ -28,11 +28,13 @@ import java.util.Map; import java.util.UUID; import java.util.stream.Collectors; +import org.apache.iceberg.expressions.Expressions; import org.apache.iceberg.relocated.com.google.common.base.Preconditions; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; import org.apache.iceberg.relocated.com.google.common.collect.Maps; +import org.apache.iceberg.types.Types; import org.apache.iceberg.util.SerializableSupplier; public class MetadataTestUtils { @@ -43,6 +45,26 @@ public class MetadataTestUtils { private static final long EXAMPLE_SEQUENCE_NUMBER = 0; private static final int EXAMPLE_SPEC_ID = 5; + private static final String TEST_LOCATION = "s3://bucket/test/location"; + + private static final Schema TEST_SCHEMA = + new Schema( + 7, + Types.NestedField.required(1, "x", Types.LongType.get()), + Types.NestedField.required(2, "y", Types.LongType.get(), "comment"), + Types.NestedField.required(3, "z", Types.LongType.get())); + + private static final long SEQ_NO = 34; + + private static final PartitionSpec SPEC_5 = + PartitionSpec.builderFor(TEST_SCHEMA).withSpecId(EXAMPLE_SPEC_ID).build(); + private static final SortOrder SORT_ORDER_3 = + SortOrder.builderFor(TEST_SCHEMA) + .withOrderId(3) + .asc("y", NullOrder.NULLS_FIRST) + .desc(Expressions.bucket("z", 4), NullOrder.NULLS_LAST) + .build(); + public static final String EXAMPLE_MANIFEST_PATH_1 = "file:/tmp/manifest1.avro"; public static final String EXAMPLE_MANIFEST_PATH_2 = "file:/tmp/manifest2.avro"; @@ -52,6 +74,21 @@ public static TableMetadataBuilder buildTestTableMetadata(int formatVersion) { return new TableMetadataBuilder(formatVersion); } + public static TableMetadataBuilder buildTestTableMetadataWithExampleValues(int formatVersion) { + return new TableMetadataBuilder(formatVersion) + .setLocation(TEST_LOCATION) + .setLastSequenceNumber(formatVersion >= 2 ? SEQ_NO : 0L) + .setLastColumnId(TEST_SCHEMA.highestFieldId()) + .setCurrentSchemaId(TEST_SCHEMA.schemaId()) + .setSchemas(ImmutableList.of(TEST_SCHEMA)) + .setDefaultSpecId(SPEC_5.specId()) + .setSpecs(ImmutableList.of(SPEC_5)) + .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) + .setDefaultSortOrderId(SORT_ORDER_3.orderId()) + .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + .setProperties(ImmutableMap.of("property", "value")); + } + public static class TableMetadataBuilder { private String metadataLocation; private int formatVersion; diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 9c99600cc845..d100a143742c 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -80,9 +80,6 @@ public class TestTableMetadata { Types.NestedField.required(2, "y", Types.LongType.get(), "comment"), Types.NestedField.required(3, "z", Types.LongType.get())); - private static final long SEQ_NO = 34; - private static final int LAST_ASSIGNED_COLUMN_ID = 3; - private static final PartitionSpec SPEC_5 = PartitionSpec.builderFor(TEST_SCHEMA).withSpecId(5).build(); private static final SortOrder SORT_ORDER_3 = @@ -101,16 +98,11 @@ public class TestTableMetadata { @SuppressWarnings("MethodLength") public void testJsonConversion(int formatVersion) throws Exception { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); - long lastSequenceNumber = formatVersion >= 2 ? SEQ_NO : 0L; Snapshot previousSnapshot = - MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) + MetadataTestUtils.buildTestSnapshotWithExampleValues() .setSnapshotId(previousSnapshotId) .setParentId(null) - .setTimestampMillis(previousSnapshotId) - .setOperation(null) - .setSummary(null) .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); @@ -120,9 +112,6 @@ public void testJsonConversion(int formatVersion) throws Exception { .setSequenceNumber(0L) .setSnapshotId(currentSnapshotId) .setParentId(previousSnapshotId) - .setTimestampMillis(currentSnapshotId) - .setOperation(null) - .setSummary(null) .setSchemaId(7) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); @@ -164,19 +153,9 @@ public void testJsonConversion(int formatVersion) throws Exception { .build()); TableMetadata expected = - MetadataTestUtils.buildTestTableMetadata(formatVersion) - .setFormatVersion(formatVersion) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(lastSequenceNumber) - .setLastColumnId(3) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(formatVersion) .setCurrentSchemaId(7) .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) - .setDefaultSpecId(5) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(3) - .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) .setSnapshotLog(snapshotLog) @@ -227,13 +206,9 @@ public void testBackwardCompat() throws Exception { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); Snapshot previousSnapshot = - MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) + MetadataTestUtils.buildTestSnapshotWithExampleValues() .setSnapshotId(previousSnapshotId) .setParentId(null) - .setTimestampMillis(previousSnapshotId) - .setOperation(null) - .setSummary(null) .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); @@ -243,20 +218,12 @@ public void testBackwardCompat() throws Exception { .setSequenceNumber(0L) .setSnapshotId(currentSnapshotId) .setParentId(previousSnapshotId) - .setTimestampMillis(currentSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); TableMetadata expected = - MetadataTestUtils.buildTestTableMetadata(1) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(1) .setFormatVersion(1) .setUuid(null) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(0) - .setLastUpdatedMillis(System.currentTimeMillis()) - .setLastColumnId(3) .setCurrentSchemaId(TableMetadata.INITIAL_SCHEMA_ID) .setSchemas(ImmutableList.of(schema)) .setDefaultSpecId(6) @@ -264,7 +231,6 @@ public void testBackwardCompat() throws Exception { .setLastAssignedPartitionId(spec.lastAssignedFieldId()) .setDefaultSortOrderId(TableMetadata.INITIAL_SORT_ORDER_ID) .setSortOrders(ImmutableList.of(sortOrder)) - .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) .build(); @@ -316,27 +282,16 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); Snapshot previousSnapshot = - MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) + MetadataTestUtils.buildTestSnapshotWithExampleValues() .setSnapshotId(previousSnapshotId) - .setParentId(null) - .setTimestampMillis(previousSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) .setSnapshotId(currentSnapshotId) .setParentId(previousSnapshotId) - .setTimestampMillis(currentSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(7) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); List snapshotLog = @@ -349,26 +304,12 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { currentSnapshot.timestampMillis(), currentSnapshot.snapshotId())) .build(); - Schema schema = new Schema(6, Types.NestedField.required(10, "x", Types.StringType.get())); - Map refs = ImmutableMap.of("main", SnapshotRef.branchBuilder(previousSnapshotId).build()); assertThatThrownBy( () -> - MetadataTestUtils.buildTestTableMetadata(formatVersion) - .setFormatVersion(formatVersion) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(SEQ_NO) - .setLastColumnId(3) - .setCurrentSchemaId(7) - .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) - .setDefaultSpecId(5) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(3) - .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .setProperties(ImmutableMap.of("property", "value")) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(formatVersion) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) .setSnapshotLog(snapshotLog) @@ -387,36 +328,17 @@ public void testMainWithoutCurrent(int formatVersion) throws IOException { Snapshot snapshot = MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) .setSnapshotId(snapshotId) - .setParentId(null) - .setTimestampMillis(snapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); - Schema schema = new Schema(6, Types.NestedField.required(10, "x", Types.StringType.get())); - Map refs = ImmutableMap.of("main", SnapshotRef.branchBuilder(snapshotId).build()); assertThatThrownBy( () -> - MetadataTestUtils.buildTestTableMetadata(formatVersion) - .setFormatVersion(formatVersion) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(SEQ_NO) - .setLastColumnId(3) - .setCurrentSchemaId(7) - .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) - .setDefaultSpecId(5) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(3) - .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .setProperties(ImmutableMap.of("property", "value")) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(formatVersion) .setSnapshots(ImmutableList.of(snapshot)) + .setCurrentSnapshotId(-1L) .setSnapshotsSupplier(null) .setRefs(refs) .build()) @@ -430,27 +352,12 @@ public void testBranchSnapshotMissing(int formatVersion) { assumeThat(formatVersion).isGreaterThanOrEqualTo(2); long snapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); - - Schema schema = new Schema(6, Types.NestedField.required(10, "x", Types.StringType.get())); - Map refs = ImmutableMap.of("main", SnapshotRef.branchBuilder(snapshotId).build()); assertThatThrownBy( () -> - MetadataTestUtils.buildTestTableMetadata(formatVersion) - .setFormatVersion(formatVersion) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(SEQ_NO) - .setLastColumnId(3) - .setCurrentSchemaId(7) - .setSchemas(ImmutableList.of(TEST_SCHEMA, schema)) - .setDefaultSpecId(5) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(3) - .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .setProperties(ImmutableMap.of("property", "value")) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(formatVersion) .setRefs(refs) .build()) .isInstanceOf(IllegalArgumentException.class) @@ -506,28 +413,16 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) .setSnapshotId(previousSnapshotId) - .setParentId(null) - .setTimestampMillis(previousSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) .setSnapshotId(currentSnapshotId) .setParentId(previousSnapshotId) - .setTimestampMillis(currentSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); - List reversedSnapshotLog = Lists.newArrayList(); long currentTimestamp = System.currentTimeMillis(); List previousMetadataLog = Lists.newArrayList(); previousMetadataLog.add( @@ -535,25 +430,9 @@ public void testJsonWithPreviousMetadataLog(int formatVersion) throws Exception currentTimestamp, "/tmp/000001-" + UUID.randomUUID() + ".metadata.json")); TableMetadata base = - MetadataTestUtils.buildTestTableMetadata(formatVersion) - .setFormatVersion(formatVersion) - .setUuid(UUID.randomUUID().toString()) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(0) - .setLastUpdatedMillis(System.currentTimeMillis()) - .setLastColumnId(3) - .setCurrentSchemaId(7) - .setSchemas(ImmutableList.of(TEST_SCHEMA)) - .setDefaultSpecId(5) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(3) - .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .setProperties(ImmutableMap.of("property", "value")) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(formatVersion) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) - .setSnapshotsSupplier(null) - .setSnapshotLog(reversedSnapshotLog) .setMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) .build(); @@ -570,26 +449,15 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) .setSnapshotId(previousSnapshotId) - .setParentId(null) - .setTimestampMillis(previousSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) .setSnapshotId(currentSnapshotId) .setParentId(previousSnapshotId) - .setTimestampMillis(currentSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); List reversedSnapshotLog = Lists.newArrayList(); @@ -611,24 +479,12 @@ public void testAddPreviousMetadataRemoveNone(int formatVersion) throws IOExcept currentTimestamp - 80, "/tmp/000003-" + UUID.randomUUID() + ".metadata.json"); TableMetadata base = - MetadataTestUtils.buildTestTableMetadata(formatVersion) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(formatVersion) .setMetadataLocation(latestPreviousMetadata.file()) - .setFormatVersion(formatVersion) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(0) - .setLastUpdatedMillis(currentTimestamp - 80) - .setLastColumnId(3) - .setCurrentSchemaId(7) - .setSchemas(ImmutableList.of(TEST_SCHEMA)) - .setDefaultSpecId(5) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(3) - .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) .setSnapshotLog(reversedSnapshotLog) + .setLastUpdatedMillis(currentTimestamp - 80) .setMetadataHistory(ImmutableList.copyOf(previousMetadataLog)) .build(); @@ -651,25 +507,14 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) .setSnapshotId(previousSnapshotId) - .setParentId(null) - .setTimestampMillis(previousSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) .setSnapshotId(currentSnapshotId) .setParentId(previousSnapshotId) - .setTimestampMillis(currentSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); List reversedSnapshotLog = Lists.newArrayList(); @@ -700,21 +545,10 @@ public void testAddPreviousMetadataRemoveOne(int formatVersion) throws IOExcepti currentTimestamp - 50, "/tmp/000006-" + UUID.randomUUID() + ".metadata.json"); TableMetadata base = - MetadataTestUtils.buildTestTableMetadata(formatVersion) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(formatVersion) .setMetadataLocation(latestPreviousMetadata.file()) - .setFormatVersion(formatVersion) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(0) .setLastUpdatedMillis(currentTimestamp - 50) .setLastColumnId(3) - .setCurrentSchemaId(7) - .setSchemas(ImmutableList.of(TEST_SCHEMA)) - .setDefaultSpecId(5) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(3) - .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) .setSnapshotLog(reversedSnapshotLog) @@ -744,25 +578,14 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx Snapshot previousSnapshot = MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) .setSnapshotId(previousSnapshotId) - .setParentId(null) - .setTimestampMillis(previousSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest1.avro")); long currentSnapshotId = System.currentTimeMillis(); Snapshot currentSnapshot = MetadataTestUtils.buildTestSnapshot() - .setSequenceNumber(0L) .setSnapshotId(currentSnapshotId) .setParentId(previousSnapshotId) - .setTimestampMillis(currentSnapshotId) - .setOperation(null) - .setSummary(null) - .setSchemaId(null) .buildWithExampleManifestList(temp, ImmutableList.of("file:/tmp/manifest2.avro")); List reversedSnapshotLog = Lists.newArrayList(); @@ -793,21 +616,9 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx currentTimestamp - 50, "/tmp/000006-" + UUID.randomUUID() + ".metadata.json"); TableMetadata base = - MetadataTestUtils.buildTestTableMetadata(formatVersion) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(formatVersion) .setMetadataLocation(latestPreviousMetadata.file()) - .setFormatVersion(formatVersion) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(0) .setLastUpdatedMillis(currentTimestamp - 50) - .setLastColumnId(3) - .setCurrentSchemaId(7) - .setSchemas(ImmutableList.of(TEST_SCHEMA)) - .setDefaultSpecId(SPEC_5.specId()) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(SortOrder.unsorted().orderId()) - .setSortOrders(ImmutableList.of(SortOrder.unsorted())) - .setProperties(ImmutableMap.of("property", "value")) .setCurrentSnapshotId(currentSnapshotId) .setSnapshots(Arrays.asList(previousSnapshot, currentSnapshot)) .setSnapshotLog(reversedSnapshotLog) @@ -837,20 +648,8 @@ public void testV2UUIDValidation(int formatVersion) { assertThatThrownBy( () -> - MetadataTestUtils.buildTestTableMetadata(formatVersion) - .setFormatVersion(formatVersion) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(formatVersion) .setUuid(null) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(SEQ_NO) - .setLastUpdatedMillis(System.currentTimeMillis()) - .setLastColumnId(LAST_ASSIGNED_COLUMN_ID) - .setCurrentSchemaId(7) - .setSchemas(ImmutableList.of(TEST_SCHEMA)) - .setDefaultSpecId(SPEC_5.specId()) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(3) - .setSortOrders(ImmutableList.of(SORT_ORDER_3)) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessage(String.format("UUID is required in format v%s", formatVersion)); @@ -862,20 +661,7 @@ public void testVersionValidation() { int unsupportedVersion = supportedVersion + 1; assertThatThrownBy( () -> - MetadataTestUtils.buildTestTableMetadata(unsupportedVersion) - .setFormatVersion(unsupportedVersion) - .setUuid(null) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(SEQ_NO) - .setLastUpdatedMillis(System.currentTimeMillis()) - .setLastColumnId(LAST_ASSIGNED_COLUMN_ID) - .setCurrentSchemaId(7) - .setSchemas(ImmutableList.of(TEST_SCHEMA)) - .setDefaultSpecId(SPEC_5.specId()) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(3) - .setSortOrders(ImmutableList.of(SORT_ORDER_3)) + MetadataTestUtils.buildTestTableMetadataWithExampleValues(unsupportedVersion) .build()) .isInstanceOf(IllegalArgumentException.class) .hasMessage( @@ -897,20 +683,7 @@ public void testVersionValidation() { unsupportedVersion, supportedVersion); // should be allowed in the supported version - assertThat( - MetadataTestUtils.buildTestTableMetadata(supportedVersion) - .setFormatVersion(supportedVersion) - .setLocation(TEST_LOCATION) - .setLastSequenceNumber(SEQ_NO) - .setLastColumnId(LAST_ASSIGNED_COLUMN_ID) - .setCurrentSchemaId(7) - .setSchemas(ImmutableList.of(TEST_SCHEMA)) - .setDefaultSpecId(SPEC_5.specId()) - .setSpecs(ImmutableList.of(SPEC_5)) - .setLastAssignedPartitionId(SPEC_5.lastAssignedFieldId()) - .setDefaultSortOrderId(3) - .setSortOrders(ImmutableList.of(SORT_ORDER_3)) - .build()) + assertThat(MetadataTestUtils.buildTestTableMetadataWithExampleValues(supportedVersion).build()) .isNotNull(); assertThat( From 34c480af42684a62e4a203161c1a935426e20329 Mon Sep 17 00:00:00 2001 From: Honah J Date: Tue, 14 Jan 2025 22:32:46 -0800 Subject: [PATCH 15/15] add constant to specify min version --- .../main/java/org/apache/iceberg/TableMetadata.java | 2 ++ .../java/org/apache/iceberg/TestTableMetadata.java | 10 ++++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java index 4ba3bdf8d737..fd623ea4436c 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadata.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java @@ -56,6 +56,8 @@ public class TableMetadata implements Serializable { static final int INITIAL_SPEC_ID = 0; static final int INITIAL_SORT_ORDER_ID = 1; static final int INITIAL_SCHEMA_ID = 0; + static final int BRANCHING_MIN_SUPPORT_VERSION = 2; + static final int UUID_REQUIRED_MIN_VERSION = 2; private static final long ONE_MINUTE = TimeUnit.MINUTES.toMillis(1); diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index d100a143742c..40ebc6e3ae11 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -18,6 +18,8 @@ */ package org.apache.iceberg; +import static org.apache.iceberg.TableMetadata.BRANCHING_MIN_SUPPORT_VERSION; +import static org.apache.iceberg.TableMetadata.UUID_REQUIRED_MIN_VERSION; import static org.apache.iceberg.TableMetadataParser.CURRENT_SNAPSHOT_ID; import static org.apache.iceberg.TableMetadataParser.FORMAT_VERSION; import static org.apache.iceberg.TableMetadataParser.LAST_COLUMN_ID; @@ -277,7 +279,7 @@ public void testBackwardCompat() throws Exception { @ParameterizedTest @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testInvalidMainBranch(int formatVersion) throws IOException { - assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + assumeThat(formatVersion).isGreaterThanOrEqualTo(BRANCHING_MIN_SUPPORT_VERSION); long previousSnapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); @@ -322,7 +324,7 @@ public void testInvalidMainBranch(int formatVersion) throws IOException { @ParameterizedTest @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") public void testMainWithoutCurrent(int formatVersion) throws IOException { - assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + assumeThat(formatVersion).isGreaterThanOrEqualTo(BRANCHING_MIN_SUPPORT_VERSION); long snapshotId = System.currentTimeMillis() - new Random(1234).nextInt(3600); @@ -643,8 +645,8 @@ public void testAddPreviousMetadataRemoveMultiple(int formatVersion) throws IOEx @ParameterizedTest @FieldSource("org.apache.iceberg.TestHelpers#ALL_VERSIONS") - public void testV2UUIDValidation(int formatVersion) { - assumeThat(formatVersion).isGreaterThanOrEqualTo(2); + public void testUUIDValidation(int formatVersion) { + assumeThat(formatVersion).isGreaterThanOrEqualTo(UUID_REQUIRED_MIN_VERSION); assertThatThrownBy( () ->