This is an automated email from the ASF dual-hosted git repository.
wgtmac pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/parquet-java.git
The following commit(s) were added to refs/heads/master by this push:
new 5e14d7cd4 GH-3637: Fix corrupt output when reusing a dictionary writer
across row groups after fallback (#3638)
5e14d7cd4 is described below
commit 5e14d7cd43403a95d331be8791e8792dbf38683d
Author: Eduard Tudenhoefner <[email protected]>
AuthorDate: Tue Jul 14 11:05:12 2026 +0200
GH-3637: Fix corrupt output when reusing a dictionary writer across row
groups after fallback (#3638)
---
.../values/dictionary/DictionaryValuesWriter.java | 1 +
.../values/fallback/FallbackValuesWriter.java | 13 ++++-
.../column/values/dictionary/TestDictionary.java | 66 +++++++++++++++++++++-
3 files changed, 75 insertions(+), 5 deletions(-)
diff --git
a/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java
b/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java
index 92e88bac9..f4ed350e3 100644
---
a/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java
+++
b/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java
@@ -202,6 +202,7 @@ public abstract class DictionaryValuesWriter extends
ValuesWriter implements Req
lastUsedDictionaryByteSize = 0;
lastUsedDictionarySize = 0;
dictionaryTooBig = false;
+ dictionaryByteSize = 0;
clearDictionaryContent();
}
diff --git
a/parquet-column/src/main/java/org/apache/parquet/column/values/fallback/FallbackValuesWriter.java
b/parquet-column/src/main/java/org/apache/parquet/column/values/fallback/FallbackValuesWriter.java
index 7f56ef219..41fe484f3 100644
---
a/parquet-column/src/main/java/org/apache/parquet/column/values/fallback/FallbackValuesWriter.java
+++
b/parquet-column/src/main/java/org/apache/parquet/column/values/fallback/FallbackValuesWriter.java
@@ -105,6 +105,11 @@ public class FallbackValuesWriter<I extends ValuesWriter &
RequiresFallback, F e
rawDataByteSize = 0;
firstPage = false;
currentWriter.reset();
+ // After a fallback, currentWriter is the fallback writer, so the initial
dictionary writer is never reset at
+ // row-group boundaries, which can silently corrupt the next row group
+ if (currentWriter != initialWriter) {
+ initialWriter.reset();
+ }
}
@Override
@@ -124,10 +129,12 @@ public class FallbackValuesWriter<I extends ValuesWriter
& RequiresFallback, F e
@Override
public void resetDictionary() {
- if (initialUsedAndHadDictionary) {
+ currentWriter.resetDictionary();
+ // After a fallback, currentWriter is the fallback writer, so the initial
dictionary writer's
+ // dictionary is never reset at row-group boundaries, leaving stale
dictionary entries/IDs that can silently
+ // corrupt the next row group
+ if (currentWriter != initialWriter) {
initialWriter.resetDictionary();
- } else {
- currentWriter.resetDictionary();
}
currentWriter = initialWriter;
fellBackAlready = false;
diff --git
a/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java
b/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java
index c7cf35199..52956a5bf 100644
---
a/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java
+++
b/parquet-column/src/test/java/org/apache/parquet/column/values/dictionary/TestDictionary.java
@@ -24,6 +24,7 @@ import static
org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.BINARY;
import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.DOUBLE;
import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.FLOAT;
import static org.apache.parquet.schema.PrimitiveType.PrimitiveTypeName.INT32;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -285,6 +286,67 @@ public class TestDictionary {
}
}
+ @Test
+ public void testDictionaryWriterReusableAfterFallBack() throws IOException {
+ int count = 1000;
+ try (final FallbackValuesWriter<PlainBinaryDictionaryValuesWriter,
PlainValuesWriter> cw =
+ newPlainBinaryDictionaryValuesWriter(1000, 10000)) {
+
+ // --- Row group 1 ---
+ // First page is dictionary encoded and committed, which keeps the
dictionary alive for
+ // the whole row group.
+ writeRepeated(count, cw, "a");
+ getBytesAndCheckEncoding(cw, PLAIN_DICTIONARY);
+
+ // Second page no longer fits the dictionary so it falls back to plain.
The current writer
+ // becomes the fallback writer, while the dictionary writer still
buffers this page's ids.
+ writeDistinct(count, cw, "b");
+ getBytesAndCheckEncoding(cw, PLAIN);
+
+ // End of row group 1: emit the dictionary page and reset the dictionary
state for reuse.
+ assertThat(cw.toDictPageAndClose()).isNotNull();
+ cw.resetDictionary();
+
+ // --- Row group 2 ---
+ // The dictionary writer must be clean again
+ writeRepeated(count, cw, "c");
+ BytesInput rg2Bytes = getBytesAndCheckEncoding(cw, PLAIN_DICTIONARY);
+
+ // The page must decode back to exactly the values written in row group
2.
+ DictionaryValuesReader cr = initDicReader(cw, BINARY);
+ checkRepeated(count, rg2Bytes, cr, "c");
+ }
+ }
+
+ @Test
+ public void testDictionaryWriterReusableAfterFirstPageFallBack() throws
IOException {
+ int count = 1000;
+ try (final FallbackValuesWriter<PlainBinaryDictionaryValuesWriter,
PlainValuesWriter> cw =
+ newPlainBinaryDictionaryValuesWriter(10000, 10000)) {
+
+ // --- Row group 1 ---
+ // The very first page falls back to plain because dictionary encoding
is not efficient. Because the
+ // fallback happens on the first page, the dictionary was never
committed as the page encoding, so
+ // initialUsedAndHadDictionary stays false and the current writer
becomes the fallback writer. The
+ // dictionary writer, however, still holds this page's entries and byte
size.
+ writeDistinct(count, cw, "a");
+ getBytesAndCheckEncoding(cw, PLAIN);
+
+ // End of row group 1: reset the dictionary state for reuse
+ cw.resetDictionary();
+
+ // --- Row group 2 ---
+ // The data is now dictionary friendly, so it must be dictionary encoded
again. Without a clean initial
+ // dictionary writer, the stale entries/byte size from row group 1 would
push this page back to plain.
+ writeRepeated(count, cw, "b");
+ BytesInput rg2Bytes = getBytesAndCheckEncoding(cw, PLAIN_DICTIONARY);
+
+ // The page must decode back to exactly the values written in row group
2.
+ DictionaryValuesReader cr = initDicReader(cw, BINARY);
+ checkRepeated(count, rg2Bytes, cr, "b");
+ }
+ }
+
@Test
public void testLongDictionary() throws IOException {
int COUNT = 1000;
@@ -827,7 +889,7 @@ public class TestDictionary {
private void checkRepeated(int COUNT, BytesInput bytes, ValuesReader cr,
String prefix) throws IOException {
cr.initFromPage(COUNT, bytes.toInputStream());
for (int i = 0; i < COUNT; i++) {
- Assert.assertEquals(prefix + i % 10, cr.readBytes().toStringUsingUTF8());
+ assertThat(cr.readBytes().toStringUsingUTF8()).isEqualTo(prefix + i %
10);
}
}
@@ -854,7 +916,7 @@ public class TestDictionary {
private BytesInput getBytesAndCheckEncoding(ValuesWriter cw, Encoding
encoding) throws IOException {
BytesInput bytes = BytesInput.copy(cw.getBytes());
- assertEquals(encoding, cw.getEncoding());
+ assertThat(cw.getEncoding()).isEqualTo(encoding);
cw.reset();
return bytes;
}
