diff --git a/parquet-column/src/main/java/org/apache/parquet/column/statistics/BinaryStatistics.java b/parquet-column/src/main/java/org/apache/parquet/column/statistics/BinaryStatistics.java index b171106804..e31c170ecc 100644 --- a/parquet-column/src/main/java/org/apache/parquet/column/statistics/BinaryStatistics.java +++ b/parquet-column/src/main/java/org/apache/parquet/column/statistics/BinaryStatistics.java @@ -57,12 +57,17 @@ public void updateStats(Binary value) { min = value.copy(); max = value.copy(); this.markAsNotEmpty(); - } else { - if (comparator().compare(min, value) > 0) { - min = value.copy(); - } else if (comparator().compare(max, value) < 0) { - max = value.copy(); - } + return; + } + // copy() keeps the same instance when the backing bytes are not reused. + // Thus, a value can be the same instance as min or max. That value is equal to the bound, so do not compare it. + if (value == min || value == max) { + return; + } + if (comparator().compare(min, value) > 0) { + min = value.copy(); + } else if (comparator().compare(max, value) < 0) { + max = value.copy(); } } diff --git a/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatistics.java b/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatistics.java index e4783b700e..83d59512db 100644 --- a/parquet-column/src/test/java/org/apache/parquet/column/statistics/TestStatistics.java +++ b/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); + stats.updateStats(Binary.fromConstantByteArray(new byte[] {20})); + assertThat(stats.genericGetMin()).isSameAs(low); + assertThat(stats.genericGetMax()).isSameAs(high); + + // A different instance with equal content does not change the bounds. Lower or higher values change them. + stats.updateStats(Binary.fromConstantByteArray(new byte[] {10})); + assertThat(stats.genericGetMin()).isSameAs(low); + assertThat(stats.genericGetMax()).isSameAs(high); + stats.updateStats(Binary.fromConstantByteArray(new byte[] {5})); + assertThat(stats.getMinBytes()).isEqualTo(new byte[] {5}); + assertThat(stats.genericGetMax()).isSameAs(high); + stats.updateStats(Binary.fromConstantByteArray(new byte[] {40})); + assertThat(stats.getMinBytes()).isEqualTo(new byte[] {5}); + assertThat(stats.getMaxBytes()).isEqualTo(new byte[] {40}); + } + @Test public void testMergingStatistics() { testMergingIntStats();