maartenbreddels commented on a change in pull request #8990:
URL: https://github.com/apache/arrow/pull/8990#discussion_r551295838
##########
File path: cpp/src/arrow/compute/kernels/scalar_string_test.cc
##########
@@ -428,6 +433,26 @@ TYPED_TEST(TestStringKernels,
StrptimeDoesNotProvideDefaultOptions) {
ASSERT_RAISES(Invalid, CallFunction("strptime", {input}));
}
+TYPED_TEST(TestStringKernels, Join) {
+ auto separator = this->scalar("--");
+ CheckScalarBinary(
+ "binary_join", list(this->type()),
+ R"([["a", "bb", "ccc"], [], null, ["dd"], ["eee", null], ["ff", ""]])",
separator,
+ this->type(), R"(["a--bb--ccc", "", null, "dd", null, "ff--"])");
Review comment:
My reasoning was that we can get that behavior using `fill_null`, while
the current behavior would otherwise be difficult to get (we don't have any
kernel that will replace list value with null if any of its list elements is
null). I'm not sure if that's a good reason to have the current behavior though.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
