andishgar commented on code in PR #50402:
URL: https://github.com/apache/arrow/pull/50402#discussion_r3637881723
##########
cpp/src/arrow/array/builder_base.h:
##########
@@ -111,6 +111,25 @@ class ARROW_EXPORT ArrayBuilder {
int num_children() const { return static_cast<int>(children_.size()); }
+ /// \brief Return true if value at index is null. Does not boundscheck
+ bool IsNull(int64_t i) const { return !IsValid(i); }
+
+ /// \brief Return true if value at index is valid (not null). Does not
+ /// boundscheck.
+ /// Note that this method does not work for types that do not have a
+ /// top-level validity bitmap (Union and Run-End Encoded (RLE) types).
+ bool IsValid(int64_t i) const {
+ switch (type()->id()) {
+ case Type::NA:
+ case Type::SPARSE_UNION:
+ case Type::DENSE_UNION:
+ case Type::RUN_END_ENCODED:
Review Comment:
@pitrou, `ArrayBuilder::IsValid` should be implemented as a virtual method.
Every time `ArrayBuilder::type()` is called, the type is constructed for
nested types. This is especially detrimental for Union Types, as it allocates
128 bytes for saving type codes on every construction.
Given this logic, `ArrayBuilder::IsValid` should be implemented via a
virtual method in each subclass. Would it be okay to enable `IsValid` for REE
and Union Types?
--
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]
