diff --git a/parquet-column/src/main/java/org/apache/parquet/column/statistics/geospatial/GeospatialStatistics.java b/parquet-column/src/main/java/org/apache/parquet/column/statistics/geospatial/GeospatialStatistics.java index c2e0529626..dc7f8c5fc0 100644 --- a/parquet-column/src/main/java/org/apache/parquet/column/statistics/geospatial/GeospatialStatistics.java +++ b/parquet-column/src/main/java/org/apache/parquet/column/statistics/geospatial/GeospatialStatistics.java @@ -43,7 +43,7 @@ public class GeospatialStatistics { public static class Builder { private BoundingBox boundingBox; private GeospatialTypes geospatialTypes; - private final WKBReader reader = new WKBReader(); + private WKBReader reader; /** * Create a builder to create a GeospatialStatistics. @@ -51,6 +51,14 @@ public static class Builder { public Builder() { this.boundingBox = new BoundingBox(); this.geospatialTypes = new GeospatialTypes(); + this.reader = new WKBReader(); + } + + /** + * Internal constructor that skips field initialization for subclasses that don't need it. + */ + Builder(boolean noop) { + // Fields intentionally left null for NoopBuilder } public void update(Binary value) { @@ -183,13 +191,19 @@ public String toString() { /** * Creates a no-op geospatial statistics builder that collects no data. * Used when geospatial statistics collection is disabled. + * This is a singleton since all methods are no-ops and it carries no state. */ private static class NoopBuilder extends Builder { - private NoopBuilder() {} + private static final NoopBuilder INSTANCE = new NoopBuilder(); + private static final GeospatialStatistics EMPTY = new GeospatialStatistics(null, null); + + private NoopBuilder() { + super(true); + } @Override public GeospatialStatistics build() { - return new GeospatialStatistics(null, null); + return EMPTY; } @Override @@ -207,6 +221,6 @@ public void abort() { * Creates a builder that doesn't collect any statistics. */ public static Builder noopBuilder() { - return new NoopBuilder(); + return NoopBuilder.INSTANCE; } } diff --git a/parquet-column/src/test/java/org/apache/parquet/column/statistics/geospatial/TestGeospatialStatistics.java b/parquet-column/src/test/java/org/apache/parquet/column/statistics/geospatial/TestGeospatialStatistics.java index 7c91ffbecb..4a1497d8a7 100644 --- a/parquet-column/src/test/java/org/apache/parquet/column/statistics/geospatial/TestGeospatialStatistics.java +++ b/parquet-column/src/test/java/org/apache/parquet/column/statistics/geospatial/TestGeospatialStatistics.java @@ -169,4 +169,30 @@ public void testNoopBuilder() { GeospatialStatistics statistics = builder.build(); Assert.assertFalse(statistics.isValid()); } + + @Test + public void testNoopBuilderIsSingleton() { + GeospatialStatistics.Builder builder1 = GeospatialStatistics.noopBuilder(); + GeospatialStatistics.Builder builder2 = GeospatialStatistics.noopBuilder(); + Assert.assertSame("noopBuilder() should return the same instance", builder1, builder2); + Assert.assertSame("build() should return the same instance", builder1.build(), builder2.build()); + } + + @Test + public void testNoopBuilderUpdateAndAbortAreNoOps() { + GeospatialStatistics.Builder builder = GeospatialStatistics.noopBuilder(); + + // update with non-null value should not throw + builder.update(Binary.fromConstantByteArray(new byte[] {0x01, 0x02, 0x03})); + // update with null should not throw + builder.update(null); + // abort should not throw + builder.abort(); + + // builder still produces an invalid (empty) result + GeospatialStatistics statistics = builder.build(); + Assert.assertFalse(statistics.isValid()); + Assert.assertNull(statistics.getBoundingBox()); + Assert.assertNull(statistics.getGeospatialTypes()); + } }