kou commented on code in PR #49443:
URL: https://github.com/apache/arrow/pull/49443#discussion_r2881274368
##########
cpp/src/arrow/compute/kernels/scalar_if_else_test.cc:
##########
@@ -3778,5 +3778,36 @@ TEST(TestChooseKernel, Errors) {
{ArrayFromJSON(int64(), "[-1]"), ArrayFromJSON(int32(),
"[0]")}));
}
+TEST_F(TestIfElseKernel, IfElseBaseBinarySlicedChunk) {
Review Comment:
Could you move this `TestIfEleseKernel` test to the location where other
`TestIfEleseKernel` tests exist instead of adding it after the
`TestChooseKernel` test?
Can we use better name than `IfElseBaseBinarySlicedChunk` like other
existing test names?
##########
cpp/src/arrow/compute/kernels/scalar_if_else_test.cc:
##########
@@ -3778,5 +3778,36 @@ TEST(TestChooseKernel, Errors) {
{ArrayFromJSON(int64(), "[-1]"), ArrayFromJSON(int32(),
"[0]")}));
}
+TEST_F(TestIfElseKernel, IfElseBaseBinarySlicedChunk) {
+ for (auto type : {utf8(), binary(), large_utf8(), large_binary()}) {
Review Comment:
Do we need to test all these types?
If so, `TYPED_TEST(TestIfElseBaseBinary, ...)` may be better than
`TEST_F(TestifEleseKernel, ...)`.
##########
cpp/src/arrow/compute/kernels/scalar_if_else_test.cc:
##########
@@ -3778,5 +3778,36 @@ TEST(TestChooseKernel, Errors) {
{ArrayFromJSON(int64(), "[-1]"), ArrayFromJSON(int32(),
"[0]")}));
}
+TEST_F(TestIfElseKernel, IfElseBaseBinarySlicedChunk) {
+ for (auto type : {utf8(), binary(), large_utf8(), large_binary()}) {
+ auto full_arr = ArrayFromJSON(type, R"([null, "x", "x", null, "x", "x"])");
+ auto chunk0 = full_arr->Slice(0, 3);
+ auto chunk1 = full_arr->Slice(3);
+
+ auto cond_asa = ArrayFromJSON(boolean(), "[true, false, false]");
+ ASSERT_OK_AND_ASSIGN(auto result_asa,
+ CallFunction("if_else", {cond_asa,
MakeNullScalar(type), chunk1}));
+ ASSERT_OK(result_asa.make_array()->ValidateFull());
+ AssertArraysEqual(*ArrayFromJSON(type, R"([null, "x", "x"])"),
+ *result_asa.make_array(), true);
+
+ auto cond_aas = ArrayFromJSON(boolean(), "[false, true, true]");
+ ASSERT_OK_AND_ASSIGN(auto result_aas,
+ CallFunction("if_else", {cond_aas, chunk1,
MakeNullScalar(type)}));
+ ASSERT_OK(result_aas.make_array()->ValidateFull());
+ AssertArraysEqual(*ArrayFromJSON(type, R"([null, "x", "x"])"),
+ *result_aas.make_array(), true);
+
+ auto arr1 = std::make_shared<ChunkedArray>(ArrayVector{chunk0, chunk1});
+ auto mask = *CallFunction("is_null", {arr1});
+ ASSERT_OK_AND_ASSIGN(auto arr2_datum,
+ CallFunction("if_else", {Datum(true),
*Concatenate(arr1->chunks()), arr1}));
+ ASSERT_OK(arr2_datum.chunked_array()->ValidateFull());
+ ASSERT_OK_AND_ASSIGN(auto arr3_datum,
+ CallFunction("if_else", {mask, MakeNullScalar(type),
arr2_datum}));
+ ASSERT_OK(arr3_datum.chunked_array()->ValidateFull());
+ AssertDatumsEqual(Datum(arr1), arr3_datum);
Review Comment:
We can reproduce this problem without chunked array, right?
If so, we don't need to test chunked array.
--
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]
