Remove unspecified bloom filter when setting properties in Iceberg

trinodb · Dec 25, 2024 · abde2be · abde2be
1 parent 4f2e41d
commit abde2be
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 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
@@ -2285,14 +2285,14 @@ public void setTableProperties(ConnectorSession session, ConnectorTableHandle ta
             List<String> parquetBloomFilterColumns = (List<String>) 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<String> 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<String> 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/...ino-iceberg/src/test/java/io/trino/plugin/iceberg/TestIcebergParquetWithBloomFilters.java b/...ino-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");