bkietz commented on a change in pull request #8728:
URL: https://github.com/apache/arrow/pull/8728#discussion_r528776241



##########
File path: cpp/src/arrow/compute/kernels/scalar_boolean.cc
##########
@@ -156,14 +307,94 @@ struct Xor {
   }
 };
 
+struct AndNot {
+  static void Call(KernelContext* ctx, const Scalar& left, const Scalar& right,
+                   Scalar* out) {
+    And::Call(ctx, left, InvertScalar(right), out);
+  }
+
+  static void Call(KernelContext* ctx, const Scalar& left, const ArrayData& 
right,
+                   ArrayData* out) {
+    if (!left.is_valid) return;  // all null case
+
+    return checked_cast<const BooleanScalar&>(left).value
+               ? GetBitmap(*out, 1).CopyFromInverted(GetBitmap(right, 1))
+               : GetBitmap(*out, 1).SetBitsTo(false);
+  }
+
+  static void Call(KernelContext* ctx, const ArrayData& left, const Scalar& 
right,
+                   ArrayData* out) {
+    And::Call(ctx, left, InvertScalar(right), out);
+  }
+
+  static void Call(KernelContext* ctx, const ArrayData& left, const ArrayData& 
right,
+                   ArrayData* out) {
+    ::arrow::internal::BitmapAndNot(left.buffers[1]->data(), left.offset,
+                                    right.buffers[1]->data(), right.offset, 
right.length,
+                                    out->offset, 
out->buffers[1]->mutable_data());
+  }
+};
+
+struct KleeneAndNot {
+  static void Call(KernelContext* ctx, const Scalar& left, const Scalar& right,
+                   Scalar* out) {
+    KleeneAnd::Call(ctx, left, InvertScalar(right), out);
+  }
+
+  static void Call(KernelContext* ctx, const Scalar& left, const ArrayData& 
right,
+                   ArrayData* out) {
+    bool left_true = left.is_valid && checked_cast<const 
BooleanScalar&>(left).value;
+    bool left_false = left.is_valid && !checked_cast<const 
BooleanScalar&>(left).value;
+
+    if (left_false) {
+      return GetBitmap(*out, 0).SetBitsTo(true),
+             GetBitmap(*out, 1).SetBitsTo(false);  // all false case
+    }
+
+    if (left_true) {
+      return GetBitmap(*out, 0).CopyFrom(GetBitmap(right, 0)),
+             GetBitmap(*out, 1).CopyFromInverted(GetBitmap(right, 1));
+    }
+
+    // scalar was null: out[i] is valid iff right[i] was true
+    ::arrow::internal::BitmapAnd(right.buffers[0]->data(), right.offset,
+                                 right.buffers[1]->data(), right.offset, 
right.length,
+                                 out->offset, out->buffers[0]->mutable_data());
+    ::arrow::internal::InvertBitmap(right.buffers[1]->data(), right.offset, 
right.length,
+                                    out->buffers[1]->mutable_data(), 
out->offset);
+  }
+
+  static void Call(KernelContext* ctx, const ArrayData& left, const Scalar& 
right,
+                   ArrayData* out) {
+    KleeneAnd::Call(ctx, left, InvertScalar(right), out);
+  }
+
+  static void Call(KernelContext* ctx, const ArrayData& left, const ArrayData& 
right,
+                   ArrayData* out) {
+    if (left.GetNullCount() == 0 && right.GetNullCount() == 0) {
+      GetBitmap(*out, 0).SetBitsTo(true);
+      return AndNot::Call(ctx, left, right, out);
+    }
+
+    static auto compute_word = [](uint64_t left_true, uint64_t left_false,
+                                  uint64_t right_true, uint64_t right_false,
+                                  uint64_t* out_valid, uint64_t* out_data) {
+      *out_data = left_true & right_false;
+      *out_valid = left_false | right_true | (left_true & right_false);

Review comment:
       `right_false` is derived from both the validity and the value bitmaps; 
it's true iff a slot was valid&false. To unpack this expression a bit: 
`and_not_kleene(left, right)` is valid if
   - `left` is false, since `and_kleene(false, x)` is false (even if `x` is 
null)
   - `right` is true, since `and_kleene(x, not true)` is false (even if `x` is 
null)
   - `left` is true AND `right` is false, since `and_kleene(true, not false)` 
is true
   
   I believe the tests exercise the full truth table of and_not_kleene so if 
this still looks suspicious then I'd look there first.




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