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 <emkornfi...@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 <emkornfi...@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,