This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new a811bcd6b50 GH-50333: [C++][Parquet] Add dense decode path for
FIXED_LEN_BYTE_ARRAY (#50335)
a811bcd6b50 is described below
commit a811bcd6b50755cc489ee43d8ede70cc8b9a7a25
Author: Marcin Krystianc <[email protected]>
AuthorDate: Wed Sep 9 16:57:22 2026 +0200
GH-50333: [C++][Parquet] Add dense decode path for FIXED_LEN_BYTE_ARRAY
(#50335)
### Rationale for this change
For performance reasons, we would like to decode float16 (FLBA) values
directly into the caller-provided buffer.
### What changes are included in this PR?
- Adds a `virtual int Decode(uint8_t* buffer, int max_values)` to
`FLBADecoder` in `encoding.h`. It writes decoded values contiguously into a
caller-owned buffer.
- Implements the new overload for all supported decoders. (The base
implementation throws ParquetException, so unimplemented encodings fail with a
clear message.)
- Replaces the old PARQUET-1508 TODO comment.
### Are these changes tested?
Yes
### Are there any user-facing changes?
No
### PS:
Closes #50333
Authored-by: Marcin Krystianc <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/parquet/decoder.cc | 74 +++++++++++++++++-
cpp/src/parquet/encoding.h | 19 ++++-
cpp/src/parquet/encoding_test.cc | 163 +++++++++++++++++++++++++++++++++++++++
3 files changed, 251 insertions(+), 5 deletions(-)
diff --git a/cpp/src/parquet/decoder.cc b/cpp/src/parquet/decoder.cc
index c4d3fe5a8a5..47f2b46ebaa 100644
--- a/cpp/src/parquet/decoder.cc
+++ b/cpp/src/parquet/decoder.cc
@@ -854,7 +854,26 @@ class PlainByteArrayDecoder : public
PlainDecoder<ByteArrayType> {
class PlainFLBADecoder : public PlainDecoder<FLBAType>, public FLBADecoder {
public:
using Base = PlainDecoder<FLBAType>;
+ using Base::Decode; // keep Decode(FixedLenByteArray*, int)
using Base::PlainDecoder;
+
+ // PLAIN-encoded FLBA values are already contiguous in the page buffer, so
+ // decode them with a single memcpy into the caller's buffer. This is the
same
+ // copy used by PlainDecoder<FLBAType>::DecodeArrow, without the builder.
+ int Decode(uint8_t* buffer, int max_values) override {
+ max_values = std::min(max_values, this->num_values_);
+ const int64_t bytes_to_decode = static_cast<int64_t>(this->type_length_) *
max_values;
+ if (bytes_to_decode > this->len_) {
+ ParquetException::EofException();
+ }
+ if (bytes_to_decode > 0) {
+ memcpy(buffer, this->data_, static_cast<size_t>(bytes_to_decode));
+ }
+ this->data_ += bytes_to_decode;
+ this->len_ -= static_cast<int>(bytes_to_decode);
+ this->num_values_ -= max_values;
+ return max_values;
+ }
};
// ----------------------------------------------------------------------
@@ -1431,6 +1450,36 @@ class DictByteArrayDecoderImpl : public
DictDecoderImpl<ByteArrayType> {
}
};
+// Dictionary decoder for FIXED_LEN_BYTE_ARRAY that can decode directly into a
+// caller-owned, densely packed byte buffer. DictDecoderImpl<FLBAType> on its
own
+// does not inherit FLBADecoder, so this thin subclass adds the dense Decode
+// overload (mirroring the DeltaByteArray and ByteStreamSplit FLBA decoders).
+class DictFLBADecoder : public DictDecoderImpl<FLBAType>, public FLBADecoder {
+ public:
+ using Base = DictDecoderImpl<FLBAType>;
+ using Base::Decode; // keep Decode(FixedLenByteArray*, int)
+ using Base::DictDecoderImpl;
+
+ // Read one index per value and copy that dictionary entry's type_length
bytes
+ // contiguously into the caller's buffer. Mirrors DecodeArrow without nulls.
+ int Decode(uint8_t* buffer, int max_values) override {
+ max_values = std::min(max_values, this->num_values_);
+ const auto* dict_values = this->dictionary_->data_as<FLBA>();
+ const int64_t type_length = this->type_length_;
+ for (int i = 0; i < max_values; ++i) {
+ int32_t index;
+ if (ARROW_PREDICT_FALSE(!this->idx_decoder_.Get(&index))) {
+ throw ParquetException("Dict decoding failed");
+ }
+ PARQUET_THROW_NOT_OK(this->IndexInBounds(index));
+ memcpy(buffer + i * type_length, dict_values[index].ptr,
+ static_cast<size_t>(type_length));
+ }
+ this->num_values_ -= max_values;
+ return max_values;
+ }
+};
+
// ----------------------------------------------------------------------
// DELTA_BINARY_PACKED decoder
@@ -2232,6 +2281,23 @@ class DeltaByteArrayFLBADecoder : public
DeltaByteArrayDecoderImpl<FLBAType>,
}
return decoded_values_size;
}
+
+ // Same internal decode as above, but copy the bytes contiguously into the
+ // caller's buffer instead of materializing per-value pointers.
+ int Decode(uint8_t* buffer, int max_values) override {
+ std::vector<ByteArray> decode_byte_array(max_values);
+ const int decoded_values_size = GetInternal(decode_byte_array.data(),
max_values);
+ const uint32_t type_length = static_cast<uint32_t>(this->type_length_);
+
+ for (int i = 0; i < decoded_values_size; i++) {
+ if (ARROW_PREDICT_FALSE(decode_byte_array[i].len != type_length)) {
+ throw ParquetException("Fixed length byte array length mismatch");
+ }
+ memcpy(buffer + static_cast<int64_t>(i) * type_length,
decode_byte_array[i].ptr,
+ type_length);
+ }
+ return decoded_values_size;
+ }
};
// ----------------------------------------------------------------------
@@ -2370,6 +2436,12 @@ class ByteStreamSplitDecoder<FLBAType> : public
ByteStreamSplitDecoderBase<FLBAT
}
return num_decoded;
}
+
+ // DecodeRaw already unsplits the byte streams into a contiguous buffer, so
+ // decode straight into the caller's buffer with no intermediate scratch.
+ int Decode(uint8_t* buffer, int max_values) override {
+ return this->DecodeRaw(buffer, max_values);
+ }
};
} // namespace
@@ -2475,7 +2547,7 @@ std::unique_ptr<Decoder> MakeDictDecoder(Type::type
type_num,
case Type::BYTE_ARRAY:
return std::make_unique<DictByteArrayDecoderImpl>(descr, pool);
case Type::FIXED_LEN_BYTE_ARRAY:
- return std::make_unique<DictDecoderImpl<FLBAType>>(descr, pool);
+ return std::make_unique<DictFLBADecoder>(descr, pool);
default:
break;
}
diff --git a/cpp/src/parquet/encoding.h b/cpp/src/parquet/encoding.h
index e3de4f2aa60..9a0cc55aa18 100644
--- a/cpp/src/parquet/encoding.h
+++ b/cpp/src/parquet/encoding.h
@@ -410,12 +410,23 @@ class BooleanDecoder : virtual public
TypedDecoder<BooleanType> {
class FLBADecoder : virtual public TypedDecoder<FLBAType> {
public:
+ using TypedDecoder<FLBAType>::Decode;
using TypedDecoder<FLBAType>::DecodeSpaced;
- // TODO(wesm): As possible follow-up to PARQUET-1508, we should examine if
- // there is value in adding specialized read methods for
- // FIXED_LEN_BYTE_ARRAY. If only Decimal data can occur with this data type
- // then perhaps not
+ /// \brief Decode values into a densely packed buffer
+ ///
+ /// Unlike Decode(FixedLenByteArray*, int), which writes one pointer per
+ /// value, this writes the raw fixed-width values back to back, with no
+ /// per-value pointers and no gaps.
+ ///
+ /// \param[in] buffer destination for decoded values; caller owns it and
+ /// must size it to at least max_values * descr->type_length() bytes.
+ /// \param[in] max_values max values to decode.
+ /// \return The number of values decoded. Should be identical to max_values
+ /// except at the end of the current data page.
+ ///
+ /// \note API EXPERIMENTAL
+ virtual int Decode(uint8_t* buffer, int max_values) = 0;
};
PARQUET_EXPORT
diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc
index 831829e4a21..74b422cb606 100644
--- a/cpp/src/parquet/encoding_test.cc
+++ b/cpp/src/parquet/encoding_test.cc
@@ -2660,4 +2660,167 @@ TEST(DeltaByteArrayEncodingAdHoc, ArrowDirectPut) {
}
}
+// ----------------------------------------------------------------------
+// Dense FIXED_LEN_BYTE_ARRAY decode tests
+//
+// FLBADecoder::Decode(uint8_t*, int) writes decoded values back to back into a
+// densely packed buffer, with no per-value FixedLenByteArray pointers. Verify
+// it for every encoding that overrides it: PLAIN, RLE_DICTIONARY,
+// DELTA_BYTE_ARRAY and BYTE_STREAM_SPLIT.
+
+class TestFLBADenseDecode : public ::testing::Test {
+ public:
+ void SetUp() override {
+ descr_ = ExampleDescr<FLBAType>();
+ type_length_ = descr_->type_length();
+ draws_.resize(kNumValues);
+ GenerateData<FLBA>(kNumValues, draws_.data(), &data_buffer_);
+ }
+
+ // Decode densely and compare each value against the original draw.
+ void CheckDenseDecode(FLBADecoder* decoder, const std::vector<int>&
decode_sizes) {
+ ASSERT_NE(nullptr, decoder);
+ std::vector<uint8_t> dense(static_cast<size_t>(type_length_) * kNumValues);
+
+ int values_decoded = 0;
+ for (int decode_size : decode_sizes) {
+ const int expected_decoded = std::min(decode_size, kNumValues -
values_decoded);
+ ASSERT_EQ(expected_decoded,
+ decoder->Decode(
+ dense.data() + static_cast<int64_t>(values_decoded) *
type_length_,
+ decode_size));
+ values_decoded += expected_decoded;
+ ASSERT_EQ(kNumValues - values_decoded, decoder->values_left());
+ }
+ ASSERT_EQ(kNumValues, values_decoded);
+ ASSERT_EQ(0, decoder->Decode(dense.data(), /*max_values=*/1));
+ ASSERT_EQ(0, decoder->values_left());
+
+ for (int i = 0; i < kNumValues; ++i) {
+ ASSERT_EQ(0, memcmp(dense.data() + static_cast<int64_t>(i) *
type_length_,
+ draws_[i].ptr, type_length_))
+ << "mismatch at value " << i;
+ }
+ }
+
+ void CheckDenseDecode(FLBADecoder* decoder) {
+ CheckDenseDecode(decoder, {1, 17, kNumValues / 2, kNumValues});
+ }
+
+ protected:
+ static constexpr int kNumValues = 1000;
+ int type_length_;
+ std::vector<FLBA> draws_;
+ std::vector<uint8_t> data_buffer_;
+ std::shared_ptr<ColumnDescriptor> descr_;
+};
+
+// These encodings are all built the same way, so exercise them with a single
+// body. RLE_DICTIONARY needs a dictionary and is tested separately below.
+TEST_F(TestFLBADenseDecode, NonDictionaryEncodings) {
+ for (auto encoding :
+ {Encoding::PLAIN, Encoding::DELTA_BYTE_ARRAY,
Encoding::BYTE_STREAM_SPLIT}) {
+ SCOPED_TRACE(EncodingToString(encoding));
+
+ auto encoder =
+ MakeTypedEncoder<FLBAType>(encoding, /*use_dictionary=*/false,
descr_.get());
+ encoder->Put(draws_.data(), kNumValues);
+ auto buffer = encoder->FlushValues();
+
+ auto decoder = MakeTypedDecoder<FLBAType>(encoding, descr_.get());
+ decoder->SetData(kNumValues, buffer->data(),
static_cast<int>(buffer->size()));
+ // EXPECT rather than ASSERT so one failing encoding doesn't hide the
others.
+
EXPECT_NO_FATAL_FAILURE(CheckDenseDecode(dynamic_cast<FLBADecoder*>(decoder.get())));
+ }
+}
+
+TEST_F(TestFLBADenseDecode, Dictionary) {
+ auto base_encoder = MakeEncoder(::parquet::Type::FIXED_LEN_BYTE_ARRAY,
Encoding::PLAIN,
+ /*use_dictionary=*/true, descr_.get());
+ auto encoder = dynamic_cast<TypedEncoder<FLBAType>*>(base_encoder.get());
+ auto dict_traits = dynamic_cast<DictEncoder<FLBAType>*>(base_encoder.get());
+
+ encoder->Put(draws_.data(), kNumValues);
+ auto dict_buffer =
+ AllocateBuffer(default_memory_pool(), dict_traits->dict_encoded_size());
+ dict_traits->WriteDict(dict_buffer->mutable_data());
+ auto indices = encoder->FlushValues();
+
+ auto dict_decoder = MakeTypedDecoder<FLBAType>(Encoding::PLAIN,
descr_.get());
+ dict_decoder->SetData(dict_traits->num_entries(), dict_buffer->data(),
+ static_cast<int>(dict_buffer->size()));
+
+ auto decoder = MakeDictDecoder<FLBAType>(descr_.get());
+ decoder->SetDict(dict_decoder.get());
+ decoder->SetData(kNumValues, indices->data(),
static_cast<int>(indices->size()));
+ // dict_decoder must outlive the decode: the decoded bytes are owned by it.
+
ASSERT_NO_FATAL_FAILURE(CheckDenseDecode(dynamic_cast<FLBADecoder*>(decoder.get())));
+}
+
+// DELTA_BYTE_ARRAY stores each value as a prefix shared with its predecessor
plus
+// a suffix, so the dense decode path has to reconstruct the two halves. The
+// random draws used above hardly ever share a prefix, which leaves that
+// reconstruction untested; use prefixed data here instead.
+TEST_F(TestFLBADenseDecode, DeltaByteArrayPrefixedData) {
+ for (double prefixed_probability : {0.5, 1.0}) {
+ SCOPED_TRACE("prefixed_probability=" +
std::to_string(prefixed_probability));
+ GeneratePrefixedData<FLBA>(kNumValues, draws_.data(), &data_buffer_,
+ prefixed_probability);
+
+ auto encoder = MakeTypedEncoder<FLBAType>(Encoding::DELTA_BYTE_ARRAY,
+ /*use_dictionary=*/false,
descr_.get());
+ encoder->Put(draws_.data(), kNumValues);
+ auto buffer = encoder->FlushValues();
+
+ auto decoder = MakeTypedDecoder<FLBAType>(Encoding::DELTA_BYTE_ARRAY,
descr_.get());
+ decoder->SetData(kNumValues, buffer->data(),
static_cast<int>(buffer->size()));
+
EXPECT_NO_FATAL_FAILURE(CheckDenseDecode(dynamic_cast<FLBADecoder*>(decoder.get())));
+ }
+}
+
+TEST_F(TestFLBADenseDecode, PlainRejectsTruncatedDenseBuffer) {
+ auto encoder =
+ MakeTypedEncoder<FLBAType>(Encoding::PLAIN, /*use_dictionary=*/false,
descr_.get());
+ encoder->Put(draws_.data(), kNumValues);
+ auto buffer = encoder->FlushValues();
+
+ auto decoder = MakeTypedDecoder<FLBAType>(Encoding::PLAIN, descr_.get());
+ decoder->SetData(kNumValues, buffer->data(), static_cast<int>(buffer->size()
- 1));
+
+ std::vector<uint8_t> dense(static_cast<size_t>(type_length_) * kNumValues);
+ ASSERT_THROW(decoder->Decode(dense.data(), kNumValues), ParquetException);
+}
+
+TEST_F(TestFLBADenseDecode, DictionaryRejectsOutOfBoundsDenseIndex) {
+ auto dict_decoder = MakeTypedDecoder<FLBAType>(Encoding::PLAIN,
descr_.get());
+ dict_decoder->SetData(/*num_values=*/1, draws_[0].ptr, type_length_);
+
+ auto decoder = MakeDictDecoder<FLBAType>(descr_.get());
+ decoder->SetDict(dict_decoder.get());
+
+ // RLE_DICTIONARY data: bit width 1, followed by an RLE run of one index
value
+ // 1. The dictionary has only one entry, so the index is out of bounds.
+ const std::vector<uint8_t> indices = {1, 2, 1};
+ decoder->SetData(/*num_values=*/1, indices.data(),
static_cast<int>(indices.size()));
+
+ auto flba_decoder = dynamic_cast<FLBADecoder*>(decoder.get());
+ ASSERT_NE(nullptr, flba_decoder);
+ std::vector<uint8_t> dense(static_cast<size_t>(type_length_));
+ ASSERT_THROW(flba_decoder->Decode(dense.data(), /*max_values=*/1),
ParquetException);
+}
+
+TEST_F(TestFLBADenseDecode, DeltaByteArrayRejectsWrongFLBALengthDense) {
+ std::string suffix(static_cast<size_t>(type_length_ - 1), 'x');
+ auto buffer =
+ ::arrow::ConcatenateBuffers({DeltaEncode({0}), DeltaEncode({type_length_
- 1}),
+ std::make_shared<Buffer>(suffix)})
+ .ValueOrDie();
+
+ auto decoder = MakeTypedDecoder<FLBAType>(Encoding::DELTA_BYTE_ARRAY,
descr_.get());
+ decoder->SetData(/*num_values=*/1, buffer->data(),
static_cast<int>(buffer->size()));
+
+ std::vector<uint8_t> dense(static_cast<size_t>(type_length_));
+ ASSERT_THROW(decoder->Decode(dense.data(), /*max_values=*/1),
ParquetException);
+}
+
} // namespace parquet::test
