Skip to content

Commit 9db6236

Browse files
authored
GH-3224: Make ParquetProperties.valuesWriterFactory thread safe (#3308)
1 parent 43c5976 commit 9db6236

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-1
lines changed

parquet-column/src/main/java/org/apache/parquet/column/ParquetProperties.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,11 @@ public class ParquetProperties {
6969

7070
public static final boolean DEFAULT_PAGE_WRITE_CHECKSUM_ENABLED = true;
7171

72+
/**
73+
* @deprecated This shared instance can cause thread safety issues when used by multiple builders concurrently.
74+
* Use {@code new DefaultValuesWriterFactory()} instead to create individual instances.
75+
*/
76+
@Deprecated
7277
public static final ValuesWriterFactory DEFAULT_VALUES_WRITER_FACTORY = new DefaultValuesWriterFactory();
7378

7479
private static final int MIN_SLAB_SIZE = 64;
@@ -396,7 +401,7 @@ public static class Builder {
396401
private int pageValueCountThreshold = DEFAULT_PAGE_VALUE_COUNT_THRESHOLD;
397402
private boolean estimateNextSizeCheck = DEFAULT_ESTIMATE_ROW_COUNT_FOR_PAGE_SIZE_CHECK;
398403
private ByteBufferAllocator allocator = new HeapByteBufferAllocator();
399-
private ValuesWriterFactory valuesWriterFactory = DEFAULT_VALUES_WRITER_FACTORY;
404+
private ValuesWriterFactory valuesWriterFactory = new DefaultValuesWriterFactory();
400405
private int columnIndexTruncateLength = DEFAULT_COLUMN_INDEX_TRUNCATE_LENGTH;
401406
private int statisticsTruncateLength = DEFAULT_STATISTICS_TRUNCATE_LENGTH;
402407
private boolean statisticsEnabled = DEFAULT_STATISTICS_ENABLED;
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.parquet.column;
20+
21+
import static org.junit.Assert.assertNotSame;
22+
23+
import java.lang.reflect.Field;
24+
import org.apache.parquet.column.values.factory.ValuesWriterFactory;
25+
import org.junit.Test;
26+
27+
public class ParquetPropertiesThreadSafetyTest {
28+
29+
@Test
30+
public void testBuilderValuesWriterFactoryNotShared() throws Exception {
31+
ParquetProperties.Builder builder1 = ParquetProperties.builder();
32+
ParquetProperties.Builder builder2 = ParquetProperties.builder();
33+
ParquetProperties.Builder builder3 = ParquetProperties.builder();
34+
35+
Field factoryField = ParquetProperties.Builder.class.getDeclaredField("valuesWriterFactory");
36+
factoryField.setAccessible(true);
37+
38+
ValuesWriterFactory factory1 = (ValuesWriterFactory) factoryField.get(builder1);
39+
ValuesWriterFactory factory2 = (ValuesWriterFactory) factoryField.get(builder2);
40+
ValuesWriterFactory factory3 = (ValuesWriterFactory) factoryField.get(builder3);
41+
42+
assertNotSame("Builder instances should not share ValuesWriterFactory instances", factory1, factory2);
43+
assertNotSame("Builder instances should not share ValuesWriterFactory instances", factory2, factory3);
44+
assertNotSame("Builder instances should not share ValuesWriterFactory instances", factory1, factory3);
45+
}
46+
}

0 commit comments

Comments
 (0)