westonpace commented on code in PR #34408:
URL: https://github.com/apache/arrow/pull/34408#discussion_r1136298871
##########
cpp/src/arrow/array/array_test.cc:
##########
@@ -78,20 +79,58 @@ class TestArray : public ::testing::Test {
MemoryPool* pool_;
};
+template <class ScalarType>
+std::shared_ptr<ScalarType> MakeScalar(typename ScalarType::ValueType value) {
+ return std::make_shared<ScalarType>(value);
+}
Review Comment:
How is this different than `MakeScalar` in `scalar.h`? If it is different,
can we add a comment because I know I'll ask the question again :laughing:
##########
cpp/src/arrow/array/data.h:
##########
@@ -33,6 +33,19 @@
namespace arrow {
class Array;
+struct ArrayData;
+
+namespace internal {
+// ----------------------------------------------------------------------
+// Null handling for types without a validity bitmap
+
+ARROW_EXPORT bool IsNullSparseUnion(const ArrayData& data, int64_t i);
+ARROW_EXPORT bool IsNullDenseUnion(const ArrayData& data, int64_t i);
+ARROW_EXPORT bool IsNullRunEndEncoded(const ArrayData& data, int64_t i);
+
+ARROW_EXPORT bool UnionMayHaveLogicalNulls(const ArrayData& data);
+ARROW_EXPORT bool RunEndEncodedMayHaveLogicalNulls(const ArrayData& data);
Review Comment:
What prevents these functions from being `.cc` only function in an anonymous
namespace? It seems like the only need is because we have functions later in
`data.h` that could be in a `.cc` file.
##########
cpp/src/arrow/array/array_test.cc:
##########
@@ -78,20 +79,58 @@ class TestArray : public ::testing::Test {
MemoryPool* pool_;
};
+template <class ScalarType>
+std::shared_ptr<ScalarType> MakeScalar(typename ScalarType::ValueType value) {
+ return std::make_shared<ScalarType>(value);
+}
+
TEST_F(TestArray, TestNullCount) {
// These are placeholders
auto data = std::make_shared<Buffer>(nullptr, 0);
auto null_bitmap = std::make_shared<Buffer>(nullptr, 0);
- std::unique_ptr<Int32Array> arr(new Int32Array(100, data, null_bitmap, 10));
+ std::shared_ptr<Int32Array> arr(new Int32Array(100, data, null_bitmap, 10));
+ ASSERT_EQ(10, arr->ComputeLogicalNullCount());
ASSERT_EQ(10, arr->null_count());
+ ASSERT_TRUE(arr->data()->MayHaveNulls());
+ ASSERT_TRUE(arr->data()->MayHaveLogicalNulls());
- std::unique_ptr<Int32Array> arr_no_nulls(new Int32Array(100, data));
+ std::shared_ptr<Int32Array> arr_no_nulls(new Int32Array(100, data));
+ ASSERT_EQ(0, arr_no_nulls->ComputeLogicalNullCount());
ASSERT_EQ(0, arr_no_nulls->null_count());
+ ASSERT_FALSE(arr_no_nulls->data()->MayHaveNulls());
+ ASSERT_FALSE(arr_no_nulls->data()->MayHaveLogicalNulls());
- std::unique_ptr<Int32Array> arr_default_null_count(
+ std::shared_ptr<Int32Array> arr_default_null_count(
new Int32Array(100, data, null_bitmap));
ASSERT_EQ(kUnknownNullCount, arr_default_null_count->data()->null_count);
+ ASSERT_TRUE(arr_default_null_count->data()->MayHaveNulls());
+ ASSERT_TRUE(arr_default_null_count->data()->MayHaveLogicalNulls());
+
+ RunEndEncodedBuilder ree_builder(pool_,
std::make_shared<Int32Builder>(pool_),
Review Comment:
Do we have existing tests like this for sparse and dense union? (given we
now have distinct paths for those)
--
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]
