This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-10569-615f0bc611679a404dcb0d777ed59dd80caa986c in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit a95308bd83b1fb7f3f7f4159fc7f5eef7fa69a08 Author: subotac <[email protected]> AuthorDate: Sun Oct 4 03:40:13 2026 +0000 fix(parquet): honor explicit V1 FLBA dictionary opt-in (#10569) # Which issue does this PR close? - Closes #10524. # Rationale for this change Parquet 1.0 writers silently ignored explicit dictionary settings for `FIXED_LEN_BYTE_ARRAY` columns, even though dictionary encoding is valid for the physical type. # What changes are included in this PR? Honor explicit global and per-column dictionary opt-ins for Parquet 1.0 FLBA columns while preserving the existing default-off behavior. Update CDC expectations for FixedSizeBinary arrays when dictionary encoding is explicitly enabled. # Are these changes tested? Yes. Added coverage for the default behavior, global opt-in, per-column override, and explicit disable cases. The focused tests, full Parquet suite, Clippy, and formatting checks pass. # Are there any user-facing changes? Callers can now explicitly enable dictionary encoding for Parquet 1.0 FLBA columns. The default behavior is unchanged. --------- Co-authored-by: Jeffrey Vo <[email protected]> --- parquet/src/column/chunker/cdc.rs | 4 ++-- parquet/src/column/writer/encoder.rs | 4 ++-- parquet/src/column/writer/mod.rs | 39 +++++++++++++++++++++++++++--------- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/parquet/src/column/chunker/cdc.rs b/parquet/src/column/chunker/cdc.rs index f524250b1b..420bdf4f47 100644 --- a/parquet/src/column/chunker/cdc.rs +++ b/parquet/src/column/chunker/cdc.rs @@ -1041,9 +1041,9 @@ mod arrow_tests { max_chunk_size: usize, expect_dictionary_page: bool, ) { - // Boolean and FixedSizeBinary never produce dictionary pages (matching C++) + // Boolean columns never produce dictionary pages. let expect_dict = match array.data_type() { - DataType::Boolean | DataType::FixedSizeBinary(_) => false, + DataType::Boolean => false, _ => expect_dictionary_page, }; assert_eq!( diff --git a/parquet/src/column/writer/encoder.rs b/parquet/src/column/writer/encoder.rs index 0eadecb8b0..cd0d2a267c 100644 --- a/parquet/src/column/writer/encoder.rs +++ b/parquet/src/column/writer/encoder.rs @@ -276,8 +276,8 @@ impl<T: DataType> ColumnValueEncoder for ColumnValueEncoderImpl<T> { props: &WriterProperties, column_props: &ResolvedColumnProperties, ) -> Result<Self> { - let dict_supported = column_props.dictionary_enabled - && has_dictionary_support(T::get_physical_type(), props); + let dict_supported = + column_props.dictionary_enabled && has_dictionary_support(T::get_physical_type()); let dict_encoder = dict_supported.then(|| DictEncoder::new(descr.clone())); // Set either main encoder or fallback encoder. diff --git a/parquet/src/column/writer/mod.rs b/parquet/src/column/writer/mod.rs index c0eef944d0..d7268422c4 100644 --- a/parquet/src/column/writer/mod.rs +++ b/parquet/src/column/writer/mod.rs @@ -1977,13 +1977,10 @@ fn fallback_encoding(kind: Type, props: &WriterProperties) -> Encoding { } /// Returns true if dictionary is supported for column writer, false otherwise. -fn has_dictionary_support(kind: Type, props: &WriterProperties) -> bool { - match (kind, props.writer_version()) { +fn has_dictionary_support(kind: Type) -> bool { + match kind { // Booleans do not support dict encoding and should use a fallback encoding. - (Type::BOOLEAN, _) => false, - // Dictionary encoding was not enabled in PARQUET 1.0 - (Type::FIXED_LEN_BYTE_ARRAY, WriterVersion::PARQUET_1_0) => false, - (Type::FIXED_LEN_BYTE_ARRAY, WriterVersion::PARQUET_2_0) => true, + Type::BOOLEAN => false, _ => true, } } @@ -2570,14 +2567,38 @@ mod tests { #[test] fn test_column_writer_default_encoding_support_fixed_len_byte_array() { + for version in [WriterVersion::PARQUET_1_0, WriterVersion::PARQUET_2_0] { + let default_props = WriterProperties::builder() + .set_writer_version(version) + .build(); + let meta = column_write_and_get_metadata::<FixedLenByteArrayType>( + default_props, + &[ByteArray::from(vec![1u8]).into()], + ); + assert_eq!(meta.dictionary_page_offset(), Some(0)); + } + check_encoding_write_support::<FixedLenByteArrayType>( WriterVersion::PARQUET_1_0, true, &[ByteArray::from(vec![1u8]).into()], - None, - &[Encoding::PLAIN, Encoding::RLE], - &[encoding_stats(PageType::DATA_PAGE, Encoding::PLAIN, 1)], + Some(0), + &[Encoding::PLAIN, Encoding::RLE, Encoding::RLE_DICTIONARY], + &[ + encoding_stats(PageType::DICTIONARY_PAGE, Encoding::PLAIN, 1), + encoding_stats(PageType::DATA_PAGE, Encoding::RLE_DICTIONARY, 1), + ], + ); + let column_props = WriterProperties::builder() + .set_writer_version(WriterVersion::PARQUET_1_0) + .set_dictionary_enabled(false) + .set_column_dictionary_enabled(ColumnPath::from("col"), true) + .build(); + let meta = column_write_and_get_metadata::<FixedLenByteArrayType>( + column_props, + &[ByteArray::from(vec![1u8]).into()], ); + assert_eq!(meta.dictionary_page_offset(), Some(0)); check_encoding_write_support::<FixedLenByteArrayType>( WriterVersion::PARQUET_1_0, false,
