pitrou commented on a change in pull request #10817:
URL: https://github.com/apache/arrow/pull/10817#discussion_r678465787
##########
File path: cpp/src/arrow/scalar_test.cc
##########
@@ -930,117 +930,134 @@ TEST(TestDictionaryScalar, Cast) {
}
}
-TEST(TestSparseUnionScalar, Basics) {
- auto ty = sparse_union({field("string", utf8()), field("number", uint64())});
+void CheckGetValidUnionScalar(const Array& arr, int64_t index, const Scalar&
expected,
+ const Scalar& expected_value) {
+ ASSERT_OK_AND_ASSIGN(auto scalar, arr.GetScalar(index));
+ ASSERT_TRUE(scalar->Equals(expected));
+
+ const auto& as_union = checked_cast<const UnionScalar&>(*scalar);
+ ASSERT_TRUE(as_union.is_valid);
+ ASSERT_TRUE(as_union.value->Equals(expected_value));
+}
- auto alpha = MakeScalar("alpha");
- auto beta = MakeScalar("beta");
- ASSERT_OK_AND_ASSIGN(auto two, MakeScalar(uint64(), 2));
+void CheckGetNullUnionScalar(const Array& arr, int64_t index) {
+ ASSERT_OK_AND_ASSIGN(auto scalar, arr.GetScalar(index));
+ ASSERT_TRUE(scalar->Equals(MakeNullScalar(arr.type())));
- auto scalar_alpha = SparseUnionScalar(alpha, ty);
- auto scalar_beta = SparseUnionScalar(beta, ty);
- auto scalar_two = SparseUnionScalar(two, ty);
+ const auto& as_union = checked_cast<const UnionScalar&>(*scalar);
+ ASSERT_FALSE(as_union.is_valid);
+ // XXX in reality, the union array doesn't have a validity bitmap.
+ // Validity is inferred from the underlying child value, which should maybe
+ // be reflected here...
+ ASSERT_EQ(as_union.value, nullptr);
Review comment:
If we want scalars to be "shaped" closely to their corresponding arrays,
do you think `as_union.is_valid` should be true here?
(note that `as_union.valid->is_valid` would be false, unlike your suggestion)
Note however that this PR doesn't change the current behaviour.
--
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]
