From b940cad70a196fc202d313f5991794607acdd551 Mon Sep 17 00:00:00 2001 From: arnavb Date: Mon, 8 Sep 2025 06:15:59 +0000 Subject: [PATCH 1/3] update --- .../parquet/column/ParquetProperties.java | 2 +- .../ParquetPropertiesThreadSafetyTest.java | 47 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 parquet-column/src/test/java/org/apache/parquet/column/ParquetPropertiesThreadSafetyTest.java diff --git a/parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java b/parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java index d3dced53df..7f6ef9a9d0 100644 --- a/parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java +++ b/parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java @@ -396,7 +396,7 @@ public static class Builder { private int pageValueCountThreshold = DEFAULT_PAGE_VALUE_COUNT_THRESHOLD; private boolean estimateNextSizeCheck = DEFAULT_ESTIMATE_ROW_COUNT_FOR_PAGE_SIZE_CHECK; private ByteBufferAllocator allocator = new HeapByteBufferAllocator(); - private ValuesWriterFactory valuesWriterFactory = DEFAULT_VALUES_WRITER_FACTORY; + private ValuesWriterFactory valuesWriterFactory = new DefaultValuesWriterFactory(); private int columnIndexTruncateLength = DEFAULT_COLUMN_INDEX_TRUNCATE_LENGTH; private int statisticsTruncateLength = DEFAULT_STATISTICS_TRUNCATE_LENGTH; private boolean statisticsEnabled = DEFAULT_STATISTICS_ENABLED; diff --git a/parquet-column/src/test/java/org/apache/parquet/column/ParquetPropertiesThreadSafetyTest.java b/parquet-column/src/test/java/org/apache/parquet/column/ParquetPropertiesThreadSafetyTest.java new file mode 100644 index 0000000000..580e427cd7 --- /dev/null +++ b/parquet-column/src/test/java/org/apache/parquet/column/ParquetPropertiesThreadSafetyTest.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.parquet.column; + +import org.apache.parquet.column.values.factory.ValuesWriterFactory; +import org.junit.Test; + +import java.lang.reflect.Field; + +import static org.junit.Assert.assertNotSame; + +public class ParquetPropertiesThreadSafetyTest { + + @Test + public void testBuilderValuesWriterFactoryNotShared() throws Exception { + ParquetProperties.Builder builder1 = ParquetProperties.builder(); + ParquetProperties.Builder builder2 = ParquetProperties.builder(); + ParquetProperties.Builder builder3 = ParquetProperties.builder(); + + Field factoryField = ParquetProperties.Builder.class.getDeclaredField("valuesWriterFactory"); + factoryField.setAccessible(true); + + ValuesWriterFactory factory1 = (ValuesWriterFactory) factoryField.get(builder1); + ValuesWriterFactory factory2 = (ValuesWriterFactory) factoryField.get(builder2); + ValuesWriterFactory factory3 = (ValuesWriterFactory) factoryField.get(builder3); + + assertNotSame("Builder instances should not share ValuesWriterFactory instances", factory1, factory2); + assertNotSame("Builder instances should not share ValuesWriterFactory instances", factory2, factory3); + assertNotSame("Builder instances should not share ValuesWriterFactory instances", factory1, factory3); + } +} From df9777be557a712b52849955ad4b35b7d6466531 Mon Sep 17 00:00:00 2001 From: arnavb Date: Mon, 8 Sep 2025 06:37:50 +0000 Subject: [PATCH 2/3] lint --- .../parquet/column/ParquetPropertiesThreadSafetyTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/parquet-column/src/test/java/org/apache/parquet/column/ParquetPropertiesThreadSafetyTest.java b/parquet-column/src/test/java/org/apache/parquet/column/ParquetPropertiesThreadSafetyTest.java index 580e427cd7..bb622f1c1f 100644 --- a/parquet-column/src/test/java/org/apache/parquet/column/ParquetPropertiesThreadSafetyTest.java +++ b/parquet-column/src/test/java/org/apache/parquet/column/ParquetPropertiesThreadSafetyTest.java @@ -18,12 +18,11 @@ */ package org.apache.parquet.column; -import org.apache.parquet.column.values.factory.ValuesWriterFactory; -import org.junit.Test; +import static org.junit.Assert.assertNotSame; import java.lang.reflect.Field; - -import static org.junit.Assert.assertNotSame; +import org.apache.parquet.column.values.factory.ValuesWriterFactory; +import org.junit.Test; public class ParquetPropertiesThreadSafetyTest { From 5eaf14f9e58e3d1d31db05779953353c53747ddc Mon Sep 17 00:00:00 2001 From: arnavb Date: Tue, 16 Sep 2025 10:02:42 +0000 Subject: [PATCH 3/3] update --- .../java/org/apache/parquet/column/ParquetProperties.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java b/parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java index 7f6ef9a9d0..f29214b458 100644 --- a/parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java +++ b/parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java @@ -69,6 +69,11 @@ public class ParquetProperties { public static final boolean DEFAULT_PAGE_WRITE_CHECKSUM_ENABLED = true; + /** + * @deprecated This shared instance can cause thread safety issues when used by multiple builders concurrently. + * Use {@code new DefaultValuesWriterFactory()} instead to create individual instances. + */ + @Deprecated public static final ValuesWriterFactory DEFAULT_VALUES_WRITER_FACTORY = new DefaultValuesWriterFactory(); private static final int MIN_SLAB_SIZE = 64;