pitrou commented on code in PR #50335:
URL: https://github.com/apache/arrow/pull/50335#discussion_r3535062010
##########
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);
Review Comment:
Perhaps we can make this a pure virtual method to enforce that it's
implemented by all FLBA-supporting encodings?
##########
cpp/src/parquet/decoder.cc:
##########
@@ -1212,6 +1231,35 @@ int DictDecoderImpl<FLBAType>::DecodeArrow(
return num_values - null_count;
}
+// 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 {
Review Comment:
Move this below the `DictDecoderImpl<Type>` method definitions below?
##########
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_ || bytes_to_decode > INT_MAX) {
Review Comment:
`len_` is an int already, so the `INT_MAX` check is not useful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
