You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2019/01/31 17:52:36 UTC
[arrow] branch master updated: ARROW-4410: [C++] Fix edge cases in
InvertKernel
This is an automated email from the ASF dual-hosted git repository.
wesm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new 7d1a9e7 ARROW-4410: [C++] Fix edge cases in InvertKernel
7d1a9e7 is described below
commit 7d1a9e738d54f5fe5659c39b22c77e01f884f747
Author: Micah Kornfield <em...@gmail.com>
AuthorDate: Thu Jan 31 11:52:30 2019 -0600
ARROW-4410: [C++] Fix edge cases in InvertKernel
(this also includes documentation fixes which should fix the build due to doxygen warnings) but they are duplciated from ARROW-4411 https://github.com/apache/arrow/pull/3518
Author: Micah Kornfield <em...@gmail.com>
Closes #3519 from emkornfield/fix_bin_edge_cases_for_real and squashes the following commits:
97e60d47 <Micah Kornfield> Fix edge-cases in Binary Kernel Invert
e77f67ff <Micah Kornfield> remove trailing whitespace
39d93f82 <Micah Kornfield> Fix documentation on util-internal as well
---
cpp/src/arrow/compute/kernels/boolean-test.cc | 12 ++++++++++++
cpp/src/arrow/compute/kernels/boolean.cc | 12 +++++++-----
cpp/src/arrow/compute/kernels/util-internal.cc | 4 ++--
cpp/src/arrow/compute/kernels/util-internal.h | 3 +++
4 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/cpp/src/arrow/compute/kernels/boolean-test.cc b/cpp/src/arrow/compute/kernels/boolean-test.cc
index 24b3c68..5f46133 100644
--- a/cpp/src/arrow/compute/kernels/boolean-test.cc
+++ b/cpp/src/arrow/compute/kernels/boolean-test.cc
@@ -130,6 +130,18 @@ TEST_F(TestBooleanKernel, Invert) {
ASSERT_TRUE(result_ca->Equals(ca2));
}
+TEST_F(TestBooleanKernel, InvertEmptyArray) {
+ auto type = boolean();
+ std::vector<std::shared_ptr<Buffer>> data_buffers(2);
+ Datum input;
+ input.value = ArrayData::Make(boolean(), 0 /* length */, std::move(data_buffers),
+ 0 /* null_count */);
+
+ Datum result;
+ ASSERT_OK(Invert(&this->ctx_, input, &result));
+ ASSERT_TRUE(result.make_array()->Equals(input.make_array()));
+}
+
TEST_F(TestBooleanKernel, And) {
vector<bool> values1 = {true, false, true, false, true, true};
vector<bool> values2 = {true, true, false, false, true, false};
diff --git a/cpp/src/arrow/compute/kernels/boolean.cc b/cpp/src/arrow/compute/kernels/boolean.cc
index 91a0e93..78ae7d4 100644
--- a/cpp/src/arrow/compute/kernels/boolean.cc
+++ b/cpp/src/arrow/compute/kernels/boolean.cc
@@ -52,7 +52,7 @@ class InvertKernel : public UnaryKernel {
// Handle validity bitmap
result->null_count = in_data.null_count;
const std::shared_ptr<Buffer>& validity_bitmap = in_data.buffers[0];
- if (in_data.offset != 0) {
+ if (in_data.offset != 0 && in_data.null_count > 0) {
DCHECK_LE(BitUtil::BytesForBits(in_data.length), validity_bitmap->size());
CopyBitmap(validity_bitmap->data(), in_data.offset, in_data.length,
result->buffers[0]->mutable_data(), kZeroDestOffset);
@@ -61,10 +61,12 @@ class InvertKernel : public UnaryKernel {
}
// Handle output data buffer
- const std::shared_ptr<Buffer>& data_buffer = in_data.buffers[1];
- DCHECK_LE(BitUtil::BytesForBits(in_data.length), data_buffer->size());
- InvertBitmap(data_buffer->data(), in_data.offset, in_data.length,
- result->buffers[1]->mutable_data(), kZeroDestOffset);
+ if (in_data.length > 0) {
+ const Buffer& data_buffer = *in_data.buffers[1];
+ DCHECK_LE(BitUtil::BytesForBits(in_data.length), data_buffer.size());
+ InvertBitmap(data_buffer.data(), in_data.offset, in_data.length,
+ result->buffers[1]->mutable_data(), kZeroDestOffset);
+ }
return Status::OK();
}
};
diff --git a/cpp/src/arrow/compute/kernels/util-internal.cc b/cpp/src/arrow/compute/kernels/util-internal.cc
index 04ee9c0..745b30c 100644
--- a/cpp/src/arrow/compute/kernels/util-internal.cc
+++ b/cpp/src/arrow/compute/kernels/util-internal.cc
@@ -179,8 +179,8 @@ Status PrimitiveAllocatingUnaryKernel::Call(FunctionContext* ctx, const Datum& i
MemoryPool* pool = ctx->memory_pool();
// Handle the validity buffer.
- if (in_data.offset == 0) {
- // Validity bitmap will be zero copied
+ if (in_data.offset == 0 || in_data.null_count <= 0) {
+ // Validity bitmap will be zero copied (or allocated when buffer is known).
data_buffers.emplace_back();
} else {
std::shared_ptr<Buffer> buffer;
diff --git a/cpp/src/arrow/compute/kernels/util-internal.h b/cpp/src/arrow/compute/kernels/util-internal.h
index 2dd8c02..2252023 100644
--- a/cpp/src/arrow/compute/kernels/util-internal.h
+++ b/cpp/src/arrow/compute/kernels/util-internal.h
@@ -46,6 +46,9 @@ namespace detail {
/// \brief Invoke the kernel on value using the ctx and store results in outputs.
///
+/// \param[in,out] ctx The function context to use when invoking the kernel.
+/// \param[in,out] kernel The kernel to execute.
+/// \param[in] value The input value to execute the kernel with.
/// \param[out] outputs One ArrayData datum for each ArrayData available in value.
ARROW_EXPORT
Status InvokeUnaryArrayKernel(FunctionContext* ctx, UnaryKernel* kernel,