From abde2be8fe7cb98b10bf2e30794ded6b27e8ca29 Mon Sep 17 00:00:00 2001 From: Yuya Ebihara Date: Wed, 25 Dec 2024 13:42:28 +0900 Subject: [PATCH] Remove unspecified bloom filter when setting properties in Iceberg --- .../io/trino/plugin/iceberg/IcebergMetadata.java | 16 ++++++++-------- .../TestIcebergParquetWithBloomFilters.java | 4 ++++ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java index e1dcdde7a750..0e3843c56f39 100644 --- a/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java +++ b/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/IcebergMetadata.java @@ -2285,14 +2285,14 @@ public void setTableProperties(ConnectorSession session, ConnectorTableHandle ta List parquetBloomFilterColumns = (List) properties.get(PARQUET_BLOOM_FILTER_COLUMNS_PROPERTY) .orElseThrow(() -> new IllegalArgumentException("The parquet_bloom_filter_columns property cannot be empty")); validateParquetBloomFilterColumns(getColumnMetadatas(SchemaParser.fromJson(table.getTableSchemaJson()), typeManager), parquetBloomFilterColumns); - if (parquetBloomFilterColumns.isEmpty()) { - icebergTable.properties().keySet().stream() - .filter(key -> key.startsWith(PARQUET_BLOOM_FILTER_COLUMN_ENABLED_PREFIX)) - .forEach(updateProperties::remove); - } - else { - parquetBloomFilterColumns.forEach(column -> updateProperties.set(PARQUET_BLOOM_FILTER_COLUMN_ENABLED_PREFIX + column, "true")); - } + + Set existingParquetBloomFilterColumns = icebergTable.properties().keySet().stream() + .filter(key -> key.startsWith(PARQUET_BLOOM_FILTER_COLUMN_ENABLED_PREFIX)) + .map(key -> key.substring(PARQUET_BLOOM_FILTER_COLUMN_ENABLED_PREFIX.length())) + .collect(toImmutableSet()); + Set removeParquetBloomFilterColumns = Sets.difference(existingParquetBloomFilterColumns, Set.copyOf(parquetBloomFilterColumns)); + removeParquetBloomFilterColumns.forEach(column -> updateProperties.remove(PARQUET_BLOOM_FILTER_COLUMN_ENABLED_PREFIX + column)); + parquetBloomFilterColumns.forEach(column -> updateProperties.set(PARQUET_BLOOM_FILTER_COLUMN_ENABLED_PREFIX + column, "true")); } if (properties.containsKey(FILE_FORMAT_PROPERTY)) { diff --git a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergParquetWithBloomFilters.java b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergParquetWithBloomFilters.java index bd85fcaa82f9..a3f4648c3a7b 100644 --- a/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergParquetWithBloomFilters.java +++ b/plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergParquetWithBloomFilters.java @@ -70,6 +70,10 @@ void testBloomFilterPropertiesArePersistedDuringSetProperties() assertUpdate("ALTER TABLE " + tableName + " SET PROPERTIES parquet_bloom_filter_columns = ARRAY['a','B']"); verifyTableProperties(tableName); + assertUpdate("ALTER TABLE " + tableName + " SET PROPERTIES parquet_bloom_filter_columns = ARRAY['a']"); + assertThat((String) computeScalar("SHOW CREATE TABLE " + tableName)) + .contains("parquet_bloom_filter_columns = ARRAY['a']"); + assertUpdate("ALTER TABLE " + tableName + " SET PROPERTIES parquet_bloom_filter_columns = ARRAY[]"); assertThat((String) computeScalar("SHOW CREATE TABLE " + tableName)) .doesNotContain("parquet_bloom_filter_columns");