pitrou commented on code in PR #50402:
URL: https://github.com/apache/arrow/pull/50402#discussion_r3535339420
##########
cpp/src/arrow/array/array_test.cc:
##########
@@ -1252,6 +1252,33 @@ TEST_F(TestBuilder, TestResizeDownsize) {
ASSERT_GE(500, builder.capacity());
ASSERT_EQ(500, builder.length());
}
+namespace {
+
+void AssertBuilderValidityAtIndices(const ArrayBuilder& builder,
+ const std::vector<int64_t>& indices,
+ const std::vector<bool>&
expected_validity) {
+ for (size_t i = 0; i < indices.size(); ++i) {
+ ASSERT_EQ(expected_validity[i], builder.IsValid(indices[i]));
+ }
+}
+
+} // namespace
+
+TEST_F(TestBuilder, TestIsNull) {
+ NullBuilder null_builder(pool_);
+ ASSERT_OK(null_builder.AppendNull());
+ ASSERT_OK(null_builder.Append(nullptr));
+ AssertBuilderValidityAtIndices(null_builder, {0, 1}, {false, false});
Review Comment:
Shouldn't this be `{true, true}`, just like `Array::IsNull`?
##########
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).
Review Comment:
It's not exactly that it does not work, it's that it doesn't account for
logical nulls.
```suggestion
/// Unlike Array::IsNull, this method does not account for logical nulls
/// for types that do not have a top-level validity bitmap
/// (Union and Run-End Encoded 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]
