wgtmac commented on code in PR #3915:
URL: https://github.com/apache/parquet-java/pull/3915#discussion_r4438477554


##########
parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatistics.java:
##########
@@ -521,6 +521,52 @@ public void testBinaryMinMaxForReusedBackingByteArray() {
     assertThat(stats.getMinBytes()).isEqualTo(new byte[] {10});
   }
 
+  @Test
+  public void testBinaryMinMaxForRepeatedSingleConstantInstance() {
+    BinaryStatistics stats = new BinaryStatistics();
+
+    Binary value = Binary.fromConstantByteArray(new byte[] {7});
+    stats.updateStats(value);
+    stats.updateStats(value);
+    stats.updateStats(value);
+
+    assertThat(stats.genericGetMin()).isSameAs(value);
+    assertThat(stats.genericGetMax()).isSameAs(value);
+  }
+
+  @Test
+  public void testBinaryMinMaxKeepsBoundInstancesForConstantValues() {
+    BinaryStatistics stats = new BinaryStatistics();
+
+    Binary low = Binary.fromConstantByteArray(new byte[] {10});
+    Binary high = Binary.fromConstantByteArray(new byte[] {30});
+    stats.updateStats(low);
+    stats.updateStats(high);
+    // The bounds keep the constant instances. An update with a bound instance 
does not change the bounds.
+    assertThat(stats.genericGetMin()).isSameAs(low);
+    assertThat(stats.genericGetMax()).isSameAs(high);
+    stats.updateStats(low);
+    assertThat(stats.genericGetMin()).isSameAs(low);
+    assertThat(stats.genericGetMax()).isSameAs(high);
+    stats.updateStats(high);
+    assertThat(stats.genericGetMin()).isSameAs(low);
+    assertThat(stats.genericGetMax()).isSameAs(high);

Review Comment:
   Could we also assert that repeating the min and max instances does not 
invoke the comparator? These tests still pass if the fast path is removed, so 
they do not protect the optimization itself.



-- 
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