raghav-reglobe commented on code in PR #16363:
URL: https://github.com/apache/iceberg/pull/16363#discussion_r3559201647


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquet.java:
##########
@@ -516,6 +341,52 @@ public void testAvroWriterRejectsVariantType() {
         .hasMessage("Avro writer does not support variant types");
   }
 
+  @Test
+  public void adaptiveBloomFilterSizingShrinksFile() throws IOException {
+    // when PARQUET_BLOOM_FILTER_ADAPTIVE_ENABLED is not set (the default), 
the writer
+    // allocates the full PARQUET_BLOOM_FILTER_MAX_BYTES buffer (4 MiB here) 
regardless
+    // of the number of distinct values written
+    long sizeWithoutAdaptive = writeWithBloomFilter(false);
+    assertThat(sizeWithoutAdaptive)
+        .as("non-adaptive file should pad the bloom filter to 
PARQUET_BLOOM_FILTER_MAX_BYTES")
+        .isGreaterThan(3_500_000L);
+
+    // with PARQUET_BLOOM_FILTER_ADAPTIVE_ENABLED, the writer picks the 
smallest candidate
+    // bloom filter that satisfies the actual number of distinct values (5) at 
the
+    // configured FPP
+    long sizeWithAdaptive = writeWithBloomFilter(true);
+    assertThat(sizeWithAdaptive)
+        .as("adaptive file should be at least 2x smaller than the non-adaptive 
file")
+        .isLessThan(sizeWithoutAdaptive / 2);
+  }
+
+  private long writeWithBloomFilter(boolean adaptiveEnabled) throws 
IOException {
+    Schema schema = new Schema(required(1, "id", Types.LongType.get()));
+
+    ImmutableMap.Builder<String, String> propsBuilder =
+        ImmutableMap.<String, String>builder()
+            .put(PARQUET_BLOOM_FILTER_COLUMN_ENABLED_PREFIX + "id", "true")
+            .put(PARQUET_BLOOM_FILTER_MAX_BYTES, "4194304");
+    if (adaptiveEnabled) {
+      propsBuilder.put(PARQUET_BLOOM_FILTER_ADAPTIVE_ENABLED, "true");
+    }

Review Comment:
   Went with the Boolean variant — `null` now means unset, so the baseline 
write genuinely exercises the not-set default path (which its comment claims), 
and true/false set the property explicitly.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to