[GitHub] [arrow] westonpace commented on a diff in pull request #14658: ARROW-18342: [C++] AsofJoinNode support for Boolean data field

[GitBox <gi...@apache.org>]

westonpace commented on code in PR #14658:
URL: https://github.com/apache/arrow/pull/14658#discussion_r1025738098


##########
cpp/src/arrow/compute/exec/asof_join_node_test.cc:
##########
@@ -91,6 +93,18 @@ Result<BatchesWithSchema> MakeBatchesFromNumString(
         ARROW_ASSIGN_OR_RAISE(Datum as_string, Cast(num_batch.values[i], utf8()));
         ARROW_ASSIGN_OR_RAISE(Datum as_type, Cast(as_string, type));
         values.push_back(as_type);
+      } else if (Type::BOOL == type->id()) {
+        // the next 4 lines compute `as_bool` as `(bool)(x - 2*(x/2))`, i.e., the low bit
+        // of `x`. Here, `x` stands for `num_batch.values[i]`, which is an `int64` value.
+        // Taking the low bit is a somewhat arbitrary way of obtaining both `true` and
+        // `false` values from the `int64` values in the test data, in order to get good
+        // testing coverage. A simple cast to a Boolean value would not get good coverage
+        // because all positive values would be cast to `true`.
+        ARROW_ASSIGN_OR_RAISE(Datum div_two, Divide(num_batch.values[i], two));
+        ARROW_ASSIGN_OR_RAISE(Datum rounded, Multiply(div_two, two));
+        ARROW_ASSIGN_OR_RAISE(Datum low_bit, Subtract(num_batch.values[i], rounded));
+        ARROW_ASSIGN_OR_RAISE(Datum as_bool, Cast(low_bit, type));

Review Comment:
   Hmm, we have a bit_wise_and kernel but I don't think there is a C++ helper function (e.g. `BitWiseAnd`) wrapping it.  This is probably fine.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org