jorisvandenbossche commented on a change in pull request #8990:
URL: https://github.com/apache/arrow/pull/8990#discussion_r551292722



##########
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:
       Another option could also be to skip nulls from the list, instead of 
skipping the list entirely if there is a null present? 
   (so eg `["eee", null]` could give `"eee"` instead of ``null``? 
   
   (don't really know what is most sensible to do, but generally for 
aggregations (and you can see a single join as a kind of aggregation?) nulls 
get skipped instead of propagated)




----------------------------------------------------------------
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]


Reply via email to