diff --git a/core/src/main/java/org/apache/iceberg/TableMetadata.java b/core/src/main/java/org/apache/iceberg/TableMetadata.java index a6f1d428f41a..d1558e4a8602 100644 --- a/core/src/main/java/org/apache/iceberg/TableMetadata.java +++ b/core/src/main/java/org/apache/iceberg/TableMetadata.java @@ -68,7 +68,7 @@ public static TableMetadata newTableMetadata( PropertyUtil.propertyAsInt( properties, TableProperties.FORMAT_VERSION, DEFAULT_TABLE_FORMAT_VERSION); return newTableMetadata( - schema, spec, sortOrder, location, unreservedProperties(properties), formatVersion); + schema, spec, sortOrder, location, persistedProperties(properties), formatVersion); } public static TableMetadata newTableMetadata( @@ -78,7 +78,7 @@ public static TableMetadata newTableMetadata( PropertyUtil.propertyAsInt( properties, TableProperties.FORMAT_VERSION, DEFAULT_TABLE_FORMAT_VERSION); return newTableMetadata( - schema, spec, sortOrder, location, unreservedProperties(properties), formatVersion); + schema, spec, sortOrder, location, persistedProperties(properties), formatVersion); } private static Map unreservedProperties(Map rawProperties) { @@ -87,6 +87,21 @@ private static Map unreservedProperties(Map rawP .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } + private static Map persistedProperties(Map rawProperties) { + Map persistedProperties = Maps.newHashMap(); + + // explicitly set defaults that apply only to new tables + persistedProperties.put( + TableProperties.PARQUET_COMPRESSION, + TableProperties.PARQUET_COMPRESSION_DEFAULT_SINCE_1_4_0); + + rawProperties.entrySet().stream() + .filter(entry -> !TableProperties.RESERVED_PROPERTIES.contains(entry.getKey())) + .forEach(entry -> persistedProperties.put(entry.getKey(), entry.getValue())); + + return persistedProperties; + } + static TableMetadata newTableMetadata( Schema schema, PartitionSpec spec, @@ -685,7 +700,7 @@ public TableMetadata buildReplacement( .setDefaultPartitionSpec(freshSpec) .setDefaultSortOrder(freshSortOrder) .setLocation(newLocation) - .setProperties(unreservedProperties(updatedProperties)) + .setProperties(persistedProperties(updatedProperties)) .build(); } diff --git a/core/src/main/java/org/apache/iceberg/TableProperties.java b/core/src/main/java/org/apache/iceberg/TableProperties.java index 03e1f3ce8897..af90303a8693 100644 --- a/core/src/main/java/org/apache/iceberg/TableProperties.java +++ b/core/src/main/java/org/apache/iceberg/TableProperties.java @@ -143,6 +143,7 @@ private TableProperties() {} public static final String PARQUET_COMPRESSION = "write.parquet.compression-codec"; public static final String DELETE_PARQUET_COMPRESSION = "write.delete.parquet.compression-codec"; public static final String PARQUET_COMPRESSION_DEFAULT = "gzip"; + public static final String PARQUET_COMPRESSION_DEFAULT_SINCE_1_4_0 = "zstd"; public static final String PARQUET_COMPRESSION_LEVEL = "write.parquet.compression-level"; public static final String DELETE_PARQUET_COMPRESSION_LEVEL = diff --git a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java index 9a75beb59d2a..7ff21e4c389b 100644 --- a/core/src/test/java/org/apache/iceberg/TestTableMetadata.java +++ b/core/src/test/java/org/apache/iceberg/TestTableMetadata.java @@ -29,6 +29,7 @@ import static org.apache.iceberg.TableMetadataParser.SCHEMA; import static org.apache.iceberg.TableMetadataParser.SNAPSHOTS; import static org.apache.iceberg.TestHelpers.assertSameSchemaList; +import static org.assertj.core.api.Assertions.assertThat; import com.fasterxml.jackson.core.JsonGenerator; import java.io.File; @@ -1457,14 +1458,10 @@ public void testCreateV2MetadataThroughTableProperty() { null, ImmutableMap.of(TableProperties.FORMAT_VERSION, "2", "key", "val")); - Assert.assertEquals( - "format version should be configured based on the format-version key", - 2, - meta.formatVersion()); - Assert.assertEquals( - "should not contain format-version in properties", - ImmutableMap.of("key", "val"), - meta.properties()); + assertThat(meta.formatVersion()).isEqualTo(2); + assertThat(meta.properties()) + .containsEntry("key", "val") + .doesNotContainKey(TableProperties.FORMAT_VERSION); } @Test @@ -1486,14 +1483,11 @@ public void testReplaceV1MetadataToV2ThroughTableProperty() { meta.location(), ImmutableMap.of(TableProperties.FORMAT_VERSION, "2", "key2", "val2")); - Assert.assertEquals( - "format version should be configured based on the format-version key", - 2, - meta.formatVersion()); - Assert.assertEquals( - "should not contain format-version but should contain old and new properties", - ImmutableMap.of("key", "val", "key2", "val2"), - meta.properties()); + assertThat(meta.formatVersion()).isEqualTo(2); + assertThat(meta.properties()) + .containsEntry("key", "val") + .containsEntry("key2", "val2") + .doesNotContainKey(TableProperties.FORMAT_VERSION); } @Test diff --git a/flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java b/flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java index d907a58ec2bc..7f47b70286f3 100644 --- a/flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java +++ b/flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java @@ -18,6 +18,8 @@ */ package org.apache.iceberg.flink; +import static org.assertj.core.api.Assertions.assertThat; + import java.util.Arrays; import java.util.Collections; import java.util.Map; @@ -48,7 +50,6 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NoSuchTableException; 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.ImmutableSet; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; import org.apache.iceberg.relocated.com.google.common.collect.Maps; @@ -121,12 +122,10 @@ public void testCreateTable() throws TableNotExistException { Assert.assertEquals( new Schema(Types.NestedField.optional(1, "id", Types.LongType.get())).asStruct(), table.schema().asStruct()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); CatalogTable catalogTable = catalogTable("tl"); Assert.assertEquals( TableSchema.builder().field("id", DataTypes.BIGINT()).build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); } @Test @@ -176,7 +175,7 @@ public void testCreateTableIfNotExists() { sql("CREATE TABLE tl(id BIGINT)"); // Assert that table does exist. - Assert.assertEquals(Maps.newHashMap(), table("tl").properties()); + assertThat(table("tl")).isNotNull(); sql("DROP TABLE tl"); AssertHelpers.assertThrows( @@ -186,15 +185,13 @@ public void testCreateTableIfNotExists() { () -> table("tl")); sql("CREATE TABLE IF NOT EXISTS tl(id BIGINT)"); - Assert.assertEquals(Maps.newHashMap(), table("tl").properties()); + assertThat(table("tl").properties()).doesNotContainKey("key"); - final Map expectedProperties = ImmutableMap.of("key", "value"); table("tl").updateProperties().set("key", "value").commit(); - Assert.assertEquals(expectedProperties, table("tl").properties()); + assertThat(table("tl").properties()).containsEntry("key", "value"); sql("CREATE TABLE IF NOT EXISTS tl(id BIGINT)"); - Assert.assertEquals( - "Should still be the old table.", expectedProperties, table("tl").properties()); + assertThat(table("tl").properties()).containsEntry("key", "value"); } @Test @@ -206,12 +203,10 @@ public void testCreateTableLike() throws TableNotExistException { Assert.assertEquals( new Schema(Types.NestedField.optional(1, "id", Types.LongType.get())).asStruct(), table.schema().asStruct()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); CatalogTable catalogTable = catalogTable("tl2"); Assert.assertEquals( TableSchema.builder().field("id", DataTypes.BIGINT()).build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); } @Test @@ -226,7 +221,6 @@ public void testCreateTableLocation() { new Schema(Types.NestedField.optional(1, "id", Types.LongType.get())).asStruct(), table.schema().asStruct()); Assert.assertEquals("file:///tmp/location", table.location()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); } @Test @@ -242,7 +236,6 @@ public void testCreatePartitionTable() throws TableNotExistException { table.schema().asStruct()); Assert.assertEquals( PartitionSpec.builderFor(table.schema()).identity("dt").build(), table.spec()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); CatalogTable catalogTable = catalogTable("tl"); Assert.assertEquals( @@ -251,7 +244,6 @@ public void testCreatePartitionTable() throws TableNotExistException { .field("dt", DataTypes.STRING()) .build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); Assert.assertEquals(Collections.singletonList("dt"), catalogTable.getPartitionKeys()); } @@ -304,7 +296,6 @@ public void testLoadTransformPartitionTable() throws TableNotExistException { CatalogTable catalogTable = catalogTable("tl"); Assert.assertEquals( TableSchema.builder().field("id", DataTypes.BIGINT()).build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); Assert.assertEquals(Collections.emptyList(), catalogTable.getPartitionKeys()); } @@ -317,12 +308,12 @@ public void testAlterTable() throws TableNotExistException { // new sql("ALTER TABLE tl SET('newK'='newV')"); properties.put("newK", "newV"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // update old sql("ALTER TABLE tl SET('oldK'='oldV2')"); properties.put("oldK", "oldV2"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // remove property CatalogTable catalogTable = catalogTable("tl"); @@ -331,7 +322,7 @@ public void testAlterTable() throws TableNotExistException { .getCatalog(getTableEnv().getCurrentCatalog()) .get() .alterTable(new ObjectPath(DATABASE, "tl"), catalogTable.copy(properties), false); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); } @Test @@ -343,12 +334,12 @@ public void testAlterTableWithPrimaryKey() throws TableNotExistException { // new sql("ALTER TABLE tl SET('newK'='newV')"); properties.put("newK", "newV"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // update old sql("ALTER TABLE tl SET('oldK'='oldV2')"); properties.put("oldK", "oldV2"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // remove property CatalogTable catalogTable = catalogTable("tl"); @@ -357,7 +348,7 @@ public void testAlterTableWithPrimaryKey() throws TableNotExistException { .getCatalog(getTableEnv().getCurrentCatalog()) .get() .alterTable(new ObjectPath(DATABASE, "tl"), catalogTable.copy(properties), false); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); } @Test diff --git a/flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java b/flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java index 49f472b7325e..4d174866ca68 100644 --- a/flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java +++ b/flink/v1.15/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java @@ -134,11 +134,11 @@ public void testCompressionParquet() throws Exception { if (initProperties.get(TableProperties.PARQUET_COMPRESSION) == null) { Assert.assertEquals( - TableProperties.PARQUET_COMPRESSION_DEFAULT, + TableProperties.PARQUET_COMPRESSION_DEFAULT_SINCE_1_4_0, resultProperties.get(TableProperties.PARQUET_COMPRESSION)); Assert.assertEquals( TableProperties.PARQUET_COMPRESSION_LEVEL_DEFAULT, - resultProperties.get(TableProperties.PARQUET_COMPRESSION_LEVEL)); + resultProperties.get(TableProperties.PARQUET_COMPRESSION_LEVEL_DEFAULT)); } else { Assert.assertEquals( initProperties.get(TableProperties.PARQUET_COMPRESSION), diff --git a/flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java b/flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java index d907a58ec2bc..7f47b70286f3 100644 --- a/flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java +++ b/flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java @@ -18,6 +18,8 @@ */ package org.apache.iceberg.flink; +import static org.assertj.core.api.Assertions.assertThat; + import java.util.Arrays; import java.util.Collections; import java.util.Map; @@ -48,7 +50,6 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NoSuchTableException; 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.ImmutableSet; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; import org.apache.iceberg.relocated.com.google.common.collect.Maps; @@ -121,12 +122,10 @@ public void testCreateTable() throws TableNotExistException { Assert.assertEquals( new Schema(Types.NestedField.optional(1, "id", Types.LongType.get())).asStruct(), table.schema().asStruct()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); CatalogTable catalogTable = catalogTable("tl"); Assert.assertEquals( TableSchema.builder().field("id", DataTypes.BIGINT()).build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); } @Test @@ -176,7 +175,7 @@ public void testCreateTableIfNotExists() { sql("CREATE TABLE tl(id BIGINT)"); // Assert that table does exist. - Assert.assertEquals(Maps.newHashMap(), table("tl").properties()); + assertThat(table("tl")).isNotNull(); sql("DROP TABLE tl"); AssertHelpers.assertThrows( @@ -186,15 +185,13 @@ public void testCreateTableIfNotExists() { () -> table("tl")); sql("CREATE TABLE IF NOT EXISTS tl(id BIGINT)"); - Assert.assertEquals(Maps.newHashMap(), table("tl").properties()); + assertThat(table("tl").properties()).doesNotContainKey("key"); - final Map expectedProperties = ImmutableMap.of("key", "value"); table("tl").updateProperties().set("key", "value").commit(); - Assert.assertEquals(expectedProperties, table("tl").properties()); + assertThat(table("tl").properties()).containsEntry("key", "value"); sql("CREATE TABLE IF NOT EXISTS tl(id BIGINT)"); - Assert.assertEquals( - "Should still be the old table.", expectedProperties, table("tl").properties()); + assertThat(table("tl").properties()).containsEntry("key", "value"); } @Test @@ -206,12 +203,10 @@ public void testCreateTableLike() throws TableNotExistException { Assert.assertEquals( new Schema(Types.NestedField.optional(1, "id", Types.LongType.get())).asStruct(), table.schema().asStruct()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); CatalogTable catalogTable = catalogTable("tl2"); Assert.assertEquals( TableSchema.builder().field("id", DataTypes.BIGINT()).build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); } @Test @@ -226,7 +221,6 @@ public void testCreateTableLocation() { new Schema(Types.NestedField.optional(1, "id", Types.LongType.get())).asStruct(), table.schema().asStruct()); Assert.assertEquals("file:///tmp/location", table.location()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); } @Test @@ -242,7 +236,6 @@ public void testCreatePartitionTable() throws TableNotExistException { table.schema().asStruct()); Assert.assertEquals( PartitionSpec.builderFor(table.schema()).identity("dt").build(), table.spec()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); CatalogTable catalogTable = catalogTable("tl"); Assert.assertEquals( @@ -251,7 +244,6 @@ public void testCreatePartitionTable() throws TableNotExistException { .field("dt", DataTypes.STRING()) .build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); Assert.assertEquals(Collections.singletonList("dt"), catalogTable.getPartitionKeys()); } @@ -304,7 +296,6 @@ public void testLoadTransformPartitionTable() throws TableNotExistException { CatalogTable catalogTable = catalogTable("tl"); Assert.assertEquals( TableSchema.builder().field("id", DataTypes.BIGINT()).build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); Assert.assertEquals(Collections.emptyList(), catalogTable.getPartitionKeys()); } @@ -317,12 +308,12 @@ public void testAlterTable() throws TableNotExistException { // new sql("ALTER TABLE tl SET('newK'='newV')"); properties.put("newK", "newV"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // update old sql("ALTER TABLE tl SET('oldK'='oldV2')"); properties.put("oldK", "oldV2"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // remove property CatalogTable catalogTable = catalogTable("tl"); @@ -331,7 +322,7 @@ public void testAlterTable() throws TableNotExistException { .getCatalog(getTableEnv().getCurrentCatalog()) .get() .alterTable(new ObjectPath(DATABASE, "tl"), catalogTable.copy(properties), false); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); } @Test @@ -343,12 +334,12 @@ public void testAlterTableWithPrimaryKey() throws TableNotExistException { // new sql("ALTER TABLE tl SET('newK'='newV')"); properties.put("newK", "newV"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // update old sql("ALTER TABLE tl SET('oldK'='oldV2')"); properties.put("oldK", "oldV2"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // remove property CatalogTable catalogTable = catalogTable("tl"); @@ -357,7 +348,7 @@ public void testAlterTableWithPrimaryKey() throws TableNotExistException { .getCatalog(getTableEnv().getCurrentCatalog()) .get() .alterTable(new ObjectPath(DATABASE, "tl"), catalogTable.copy(properties), false); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); } @Test diff --git a/flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java b/flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java index 49f472b7325e..3f1172a19cc0 100644 --- a/flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java +++ b/flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java @@ -134,7 +134,7 @@ public void testCompressionParquet() throws Exception { if (initProperties.get(TableProperties.PARQUET_COMPRESSION) == null) { Assert.assertEquals( - TableProperties.PARQUET_COMPRESSION_DEFAULT, + TableProperties.PARQUET_COMPRESSION_DEFAULT_SINCE_1_4_0, resultProperties.get(TableProperties.PARQUET_COMPRESSION)); Assert.assertEquals( TableProperties.PARQUET_COMPRESSION_LEVEL_DEFAULT, diff --git a/flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java b/flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java index db83cea1e536..16dcf4a9f4ce 100644 --- a/flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java +++ b/flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/TestFlinkCatalogTable.java @@ -18,6 +18,8 @@ */ package org.apache.iceberg.flink; +import static org.assertj.core.api.Assertions.assertThat; + import java.util.Arrays; import java.util.Collections; import java.util.Map; @@ -47,7 +49,6 @@ import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.NoSuchTableException; 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.ImmutableSet; import org.apache.iceberg.relocated.com.google.common.collect.Iterables; import org.apache.iceberg.relocated.com.google.common.collect.Maps; @@ -121,12 +122,10 @@ public void testCreateTable() throws TableNotExistException { Assert.assertEquals( new Schema(Types.NestedField.optional(1, "id", Types.LongType.get())).asStruct(), table.schema().asStruct()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); CatalogTable catalogTable = catalogTable("tl"); Assert.assertEquals( TableSchema.builder().field("id", DataTypes.BIGINT()).build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); } @Test @@ -176,7 +175,7 @@ public void testCreateTableIfNotExists() { sql("CREATE TABLE tl(id BIGINT)"); // Assert that table does exist. - Assert.assertEquals(Maps.newHashMap(), table("tl").properties()); + assertThat(table("tl")).isNotNull(); sql("DROP TABLE tl"); Assertions.assertThatThrownBy(() -> table("tl")) @@ -184,15 +183,13 @@ public void testCreateTableIfNotExists() { .hasMessage("Table does not exist: " + getFullQualifiedTableName("tl")); sql("CREATE TABLE IF NOT EXISTS tl(id BIGINT)"); - Assert.assertEquals(Maps.newHashMap(), table("tl").properties()); + assertThat(table("tl").properties()).doesNotContainKey("key"); - final Map expectedProperties = ImmutableMap.of("key", "value"); table("tl").updateProperties().set("key", "value").commit(); - Assert.assertEquals(expectedProperties, table("tl").properties()); + assertThat(table("tl").properties()).containsEntry("key", "value"); sql("CREATE TABLE IF NOT EXISTS tl(id BIGINT)"); - Assert.assertEquals( - "Should still be the old table.", expectedProperties, table("tl").properties()); + assertThat(table("tl").properties()).containsEntry("key", "value"); } @Test @@ -204,12 +201,10 @@ public void testCreateTableLike() throws TableNotExistException { Assert.assertEquals( new Schema(Types.NestedField.optional(1, "id", Types.LongType.get())).asStruct(), table.schema().asStruct()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); CatalogTable catalogTable = catalogTable("tl2"); Assert.assertEquals( TableSchema.builder().field("id", DataTypes.BIGINT()).build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); } @Test @@ -224,7 +219,6 @@ public void testCreateTableLocation() { new Schema(Types.NestedField.optional(1, "id", Types.LongType.get())).asStruct(), table.schema().asStruct()); Assert.assertEquals("file:///tmp/location", table.location()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); } @Test @@ -240,7 +234,6 @@ public void testCreatePartitionTable() throws TableNotExistException { table.schema().asStruct()); Assert.assertEquals( PartitionSpec.builderFor(table.schema()).identity("dt").build(), table.spec()); - Assert.assertEquals(Maps.newHashMap(), table.properties()); CatalogTable catalogTable = catalogTable("tl"); Assert.assertEquals( @@ -249,7 +242,6 @@ public void testCreatePartitionTable() throws TableNotExistException { .field("dt", DataTypes.STRING()) .build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); Assert.assertEquals(Collections.singletonList("dt"), catalogTable.getPartitionKeys()); } @@ -301,7 +293,6 @@ public void testLoadTransformPartitionTable() throws TableNotExistException { CatalogTable catalogTable = catalogTable("tl"); Assert.assertEquals( TableSchema.builder().field("id", DataTypes.BIGINT()).build(), catalogTable.getSchema()); - Assert.assertEquals(Maps.newHashMap(), catalogTable.getOptions()); Assert.assertEquals(Collections.emptyList(), catalogTable.getPartitionKeys()); } @@ -314,12 +305,12 @@ public void testAlterTable() throws TableNotExistException { // new sql("ALTER TABLE tl SET('newK'='newV')"); properties.put("newK", "newV"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // update old sql("ALTER TABLE tl SET('oldK'='oldV2')"); properties.put("oldK", "oldV2"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // remove property CatalogTable catalogTable = catalogTable("tl"); @@ -328,7 +319,7 @@ public void testAlterTable() throws TableNotExistException { .getCatalog(getTableEnv().getCurrentCatalog()) .get() .alterTable(new ObjectPath(DATABASE, "tl"), catalogTable.copy(properties), false); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); } @Test @@ -340,12 +331,12 @@ public void testAlterTableWithPrimaryKey() throws TableNotExistException { // new sql("ALTER TABLE tl SET('newK'='newV')"); properties.put("newK", "newV"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // update old sql("ALTER TABLE tl SET('oldK'='oldV2')"); properties.put("oldK", "oldV2"); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); // remove property CatalogTable catalogTable = catalogTable("tl"); @@ -354,7 +345,7 @@ public void testAlterTableWithPrimaryKey() throws TableNotExistException { .getCatalog(getTableEnv().getCurrentCatalog()) .get() .alterTable(new ObjectPath(DATABASE, "tl"), catalogTable.copy(properties), false); - Assert.assertEquals(properties, table("tl").properties()); + assertThat(table("tl").properties()).containsAllEntriesOf(properties); } @Test diff --git a/flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java b/flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java index 49f472b7325e..f5bf2378a56f 100644 --- a/flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java +++ b/flink/v1.17/flink/src/test/java/org/apache/iceberg/flink/sink/TestCompressionSettings.java @@ -134,11 +134,11 @@ public void testCompressionParquet() throws Exception { if (initProperties.get(TableProperties.PARQUET_COMPRESSION) == null) { Assert.assertEquals( - TableProperties.PARQUET_COMPRESSION_DEFAULT, + TableProperties.PARQUET_COMPRESSION_DEFAULT_SINCE_1_4_0, resultProperties.get(TableProperties.PARQUET_COMPRESSION)); Assert.assertEquals( TableProperties.PARQUET_COMPRESSION_LEVEL_DEFAULT, - resultProperties.get(TableProperties.PARQUET_COMPRESSION_LEVEL)); + resultProperties.get(TableProperties.PARQUET_COMPRESSION_DEFAULT_SINCE_1_4_0)); } else { Assert.assertEquals( initProperties.get(TableProperties.PARQUET_COMPRESSION), diff --git a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java index 6b06c33a730b..7ff2bd78a665 100644 --- a/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java +++ b/hive-metastore/src/test/java/org/apache/iceberg/hive/TestHiveCatalog.java @@ -124,6 +124,11 @@ public void testCreateTableBuilder() throws Exception { assertThat(table.spec().fields()).hasSize(1); assertThat(table.properties()).containsEntry("key1", "value1"); assertThat(table.properties()).containsEntry("key2", "value2"); + // default Parquet compression is explicitly set for new tables + assertThat(table.properties()) + .containsEntry( + TableProperties.PARQUET_COMPRESSION, + TableProperties.PARQUET_COMPRESSION_DEFAULT_SINCE_1_4_0); } finally { catalog.dropTable(tableIdent); } @@ -146,6 +151,11 @@ public void testCreateTableWithCaching() throws Exception { assertThat(table.spec().fields()).hasSize(1); assertThat(table.properties()).containsEntry("key1", "value1"); assertThat(table.properties()).containsEntry("key2", "value2"); + // default Parquet compression is explicitly set for new tables + assertThat(table.properties()) + .containsEntry( + TableProperties.PARQUET_COMPRESSION, + TableProperties.PARQUET_COMPRESSION_DEFAULT_SINCE_1_4_0); } finally { cachingCatalog.dropTable(tableIdent); } diff --git a/mr/src/test/java/org/apache/iceberg/mr/TestCatalogs.java b/mr/src/test/java/org/apache/iceberg/mr/TestCatalogs.java index dba1994aec96..f849163acc6a 100644 --- a/mr/src/test/java/org/apache/iceberg/mr/TestCatalogs.java +++ b/mr/src/test/java/org/apache/iceberg/mr/TestCatalogs.java @@ -19,9 +19,9 @@ package org.apache.iceberg.mr; import static org.apache.iceberg.types.Types.NestedField.required; +import static org.assertj.core.api.Assertions.assertThat; import java.io.IOException; -import java.util.Collections; import java.util.Optional; import java.util.Properties; import org.apache.hadoop.conf.Configuration; @@ -126,7 +126,7 @@ public void testCreateDropTableToLocation() throws IOException { Assert.assertEquals(properties.getProperty("location"), table.location()); Assert.assertEquals(SchemaParser.toJson(SCHEMA), SchemaParser.toJson(table.schema())); Assert.assertEquals(PartitionSpecParser.toJson(SPEC), PartitionSpecParser.toJson(table.spec())); - Assert.assertEquals(Collections.singletonMap("dummy", "test"), table.properties()); + assertThat(table.properties()).containsEntry("dummy", "test"); Assertions.assertThatThrownBy(() -> Catalogs.dropTable(conf, new Properties())) .isInstanceOf(NullPointerException.class) @@ -178,7 +178,7 @@ public void testCreateDropTableToCatalog() throws IOException { Assert.assertEquals(SchemaParser.toJson(SCHEMA), SchemaParser.toJson(table.schema())); Assert.assertEquals(PartitionSpecParser.toJson(SPEC), PartitionSpecParser.toJson(table.spec())); - Assert.assertEquals(Collections.singletonMap("dummy", "test"), table.properties()); + assertThat(table.properties()).containsEntry("dummy", "test"); Assertions.assertThatThrownBy(() -> Catalogs.dropTable(conf, new Properties())) .isInstanceOf(NullPointerException.class) diff --git a/mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java b/mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java index 775ecbffe109..81e2ffcc84da 100644 --- a/mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java +++ b/mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java @@ -759,6 +759,9 @@ public void testIcebergAndHmsTableProperties() throws Exception { expectedIcebergProperties.put("custom_property", "initial_val"); expectedIcebergProperties.put("EXTERNAL", "TRUE"); expectedIcebergProperties.put("storage_handler", HiveIcebergStorageHandler.class.getName()); + expectedIcebergProperties.put( + TableProperties.PARQUET_COMPRESSION, + TableProperties.PARQUET_COMPRESSION_DEFAULT_SINCE_1_4_0); // Check the HMS table parameters org.apache.hadoop.hive.metastore.api.Table hmsTable = @@ -779,7 +782,7 @@ public void testIcebergAndHmsTableProperties() throws Exception { Assert.assertEquals(expectedIcebergProperties, icebergTable.properties()); if (Catalogs.hiveCatalog(shell.getHiveConf(), tableProperties)) { - Assert.assertEquals(13, hmsParams.size()); + Assert.assertEquals(14, hmsParams.size()); Assert.assertEquals("initial_val", hmsParams.get("custom_property")); Assert.assertEquals("TRUE", hmsParams.get(InputFormatConfig.EXTERNAL_TABLE_PURGE)); Assert.assertEquals("TRUE", hmsParams.get("EXTERNAL")); @@ -824,8 +827,8 @@ public void testIcebergAndHmsTableProperties() throws Exception { .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); if (Catalogs.hiveCatalog(shell.getHiveConf(), tableProperties)) { - Assert.assertEquals( - 16, hmsParams.size()); // 2 newly-added properties + previous_metadata_location prop + // 2 newly-added properties + previous_metadata_location prop + explicit Parquet compression + Assert.assertEquals(17, hmsParams.size()); Assert.assertEquals("true", hmsParams.get("new_prop_1")); Assert.assertEquals("false", hmsParams.get("new_prop_2")); Assert.assertEquals("new_val", hmsParams.get("custom_property")); diff --git a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java index 416d5eed5b65..5497017460e2 100644 --- a/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java +++ b/spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java @@ -43,6 +43,7 @@ import org.apache.iceberg.spark.SparkCatalogConfig; import org.apache.iceberg.spark.SparkTestBaseWithCatalog; import org.apache.iceberg.types.Types; +import org.apache.iceberg.util.Pair; import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.junit.After; @@ -129,7 +130,7 @@ private Table createPrimitiveTable() throws IOException { return table; } - private void createNestedTable() throws IOException { + private Pair createNestedTable() throws IOException { Table table = catalog.createTable( TableIdentifier.of(Namespace.of(database()), tableName()), @@ -145,6 +146,7 @@ private void createNestedTable() throws IOException { DataFile dataFile = FileHelpers.writeDataFile(table, Files.localOutput(temp.newFile()), records); table.newAppend().appendFile(dataFile).commit(); + return Pair.of(table, dataFile); } @After @@ -324,15 +326,22 @@ public void testSelectNestedValues() throws Exception { @Test public void testNestedValues() throws Exception { - createNestedTable(); - - Object[] leafDoubleCol = row(53L, 3L, 1L, 1L, 0.0D, 0.0D); - Object[] leafLongCol = row(54L, 3L, 1L, null, 0L, 1L); + Pair table = createNestedTable(); + int longColId = + table.first().schema().findField("nestedStructCol.leafStructCol.leafLongCol").fieldId(); + int doubleColId = + table.first().schema().findField("nestedStructCol.leafStructCol.leafDoubleCol").fieldId(); + + Object[] leafDoubleCol = + row(table.second().columnSizes().get(doubleColId), 3L, 1L, 1L, 0.0D, 0.0D); + Object[] leafLongCol = row(table.second().columnSizes().get(longColId), 3L, 1L, null, 0L, 1L); Object[] metrics = row(leafDoubleCol, leafLongCol); - assertEquals( - "Row should match", - ImmutableList.of(new Object[] {metrics}), - sql("SELECT readable_metrics FROM %s.files", tableName)); + List expected = ImmutableList.of(new Object[] {metrics}); + String sql = "SELECT readable_metrics FROM %s.%s"; + List filesReadableMetrics = sql(String.format(sql, tableName, "files")); + List entriesReadableMetrics = sql(String.format(sql, tableName, "entries")); + assertEquals("Row should match for files table", expected, filesReadableMetrics); + assertEquals("Row should match for entries table", expected, entriesReadableMetrics); } } diff --git a/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java b/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java index f65da4574284..9075257fa9f1 100644 --- a/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java +++ b/spark/v3.4/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java @@ -43,6 +43,7 @@ import org.apache.iceberg.spark.SparkCatalogConfig; import org.apache.iceberg.spark.SparkTestBaseWithCatalog; import org.apache.iceberg.types.Types; +import org.apache.iceberg.util.Pair; import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.junit.After; @@ -129,7 +130,7 @@ private Table createPrimitiveTable() throws IOException { return table; } - private void createNestedTable() throws IOException { + private Pair createNestedTable() throws IOException { Table table = catalog.createTable( TableIdentifier.of(Namespace.of(database()), tableName()), @@ -145,6 +146,7 @@ private void createNestedTable() throws IOException { DataFile dataFile = FileHelpers.writeDataFile(table, Files.localOutput(temp.newFile()), records); table.newAppend().appendFile(dataFile).commit(); + return Pair.of(table, dataFile); } @After @@ -351,10 +353,15 @@ public void testSelectNestedValues() throws Exception { @Test public void testNestedValues() throws Exception { - createNestedTable(); - - Object[] leafDoubleCol = row(53L, 3L, 1L, 1L, 0.0D, 0.0D); - Object[] leafLongCol = row(54L, 3L, 1L, null, 0L, 1L); + Pair table = createNestedTable(); + int longColId = + table.first().schema().findField("nestedStructCol.leafStructCol.leafLongCol").fieldId(); + int doubleColId = + table.first().schema().findField("nestedStructCol.leafStructCol.leafDoubleCol").fieldId(); + + Object[] leafDoubleCol = + row(table.second().columnSizes().get(doubleColId), 3L, 1L, 1L, 0.0D, 0.0D); + Object[] leafLongCol = row(table.second().columnSizes().get(longColId), 3L, 1L, null, 0L, 1L); Object[] metrics = row(leafDoubleCol, leafLongCol); List expected = ImmutableList.of(new Object[] {metrics}); diff --git a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java index f65da4574284..9075257fa9f1 100644 --- a/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java +++ b/spark/v3.5/spark/src/test/java/org/apache/iceberg/spark/source/TestMetadataTableReadableMetrics.java @@ -43,6 +43,7 @@ import org.apache.iceberg.spark.SparkCatalogConfig; import org.apache.iceberg.spark.SparkTestBaseWithCatalog; import org.apache.iceberg.types.Types; +import org.apache.iceberg.util.Pair; import org.apache.spark.sql.Dataset; import org.apache.spark.sql.Row; import org.junit.After; @@ -129,7 +130,7 @@ private Table createPrimitiveTable() throws IOException { return table; } - private void createNestedTable() throws IOException { + private Pair createNestedTable() throws IOException { Table table = catalog.createTable( TableIdentifier.of(Namespace.of(database()), tableName()), @@ -145,6 +146,7 @@ private void createNestedTable() throws IOException { DataFile dataFile = FileHelpers.writeDataFile(table, Files.localOutput(temp.newFile()), records); table.newAppend().appendFile(dataFile).commit(); + return Pair.of(table, dataFile); } @After @@ -351,10 +353,15 @@ public void testSelectNestedValues() throws Exception { @Test public void testNestedValues() throws Exception { - createNestedTable(); - - Object[] leafDoubleCol = row(53L, 3L, 1L, 1L, 0.0D, 0.0D); - Object[] leafLongCol = row(54L, 3L, 1L, null, 0L, 1L); + Pair table = createNestedTable(); + int longColId = + table.first().schema().findField("nestedStructCol.leafStructCol.leafLongCol").fieldId(); + int doubleColId = + table.first().schema().findField("nestedStructCol.leafStructCol.leafDoubleCol").fieldId(); + + Object[] leafDoubleCol = + row(table.second().columnSizes().get(doubleColId), 3L, 1L, 1L, 0.0D, 0.0D); + Object[] leafLongCol = row(table.second().columnSizes().get(longColId), 3L, 1L, null, 0L, 1L); Object[] metrics = row(leafDoubleCol, leafLongCol); List expected = ImmutableList.of(new Object[] {metrics});