zanmato1984 commented on code in PR #50843:
URL: https://github.com/apache/arrow/pull/50843#discussion_r3752820795
##########
cpp/src/arrow/type.h:
##########
@@ -196,6 +197,22 @@ class ARROW_EXPORT DataType : public
std::enable_shared_from_this<DataType>,
/// subclasses of FixedWidthType
virtual int bit_width() const { return -1; }
+ /// \brief Returns the number of bytes needed to store `num_elements`
+ /// values of this fixed-width type, rounding up for bit-packed types
+ /// (e.g. boolean) that use less than one byte per value. Returns -1
+ /// for non-fixed-width types, and should only be used for subclasses
+ /// of FixedWidthType
+ virtual int64_t bytes_required(int64_t num_elements) const {
Review Comment:
Could we avoid adding a new public virtual API on `DataType` for this? This
code is already in an `is_fixed_width(...)` path, and Arrow already has
existing patterns that size fixed-width buffers from
`FixedWidthType::bit_width()` plus `bit_util::BytesForBits(length *
bit_width)`, which also handles bit-packed boolean correctly.
Maybe add a small local helper in `vector_replace.cc` and use it for
`FillNullForwardChunked` / `FillNullBackwardChunked`, and also
`ReplaceMaskChunked`, which still has the same `byte_width() * chunk->length()`
issue for chunked boolean arrays.
--
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]
