From 9ad362695c54a8a74d00ed907aa37521df0b59e6 Mon Sep 17 00:00:00 2001
From: "Maarten A. Breddels" <maartenbreddels@gmail.com>
Date: Tue, 10 Nov 2020 17:30:01 +0100
Subject: [PATCH 01/11] ARROW-9489: [C++] Add fill_null kernel implementation
 for (array[string], scalar[string])

---
 .../arrow/compute/kernels/codegen_internal.h  | 16 ++++++
 .../arrow/compute/kernels/scalar_fill_null.cc | 56 +++++++++++++++++++
 .../compute/kernels/scalar_fill_null_test.cc  | 10 ++++
 python/pyarrow/tests/test_compute.py          | 11 ++++
 4 files changed, 93 insertions(+)

diff --git a/cpp/src/arrow/compute/kernels/codegen_internal.h b/cpp/src/arrow/compute/kernels/codegen_internal.h
index 1f940249857..abcb5b9de1d 100644
--- a/cpp/src/arrow/compute/kernels/codegen_internal.h
+++ b/cpp/src/arrow/compute/kernels/codegen_internal.h
@@ -1058,6 +1058,22 @@ ArrayKernelExec GenerateTypeAgnosticPrimitive(detail::GetTypeId get_id) {
   }
 }
 
+/// similar to GenerateTypeAgnosticPrimitive, but for variable types
+template <template <typename...> class Generator>
+ArrayKernelExec GenerateTypeAgnosticVarBinaryBase(detail::GetTypeId get_id) {
+  switch (get_id.id) {
+    case Type::BINARY:
+    case Type::STRING:
+      return Generator<BinaryType>::Exec;
+    case Type::LARGE_BINARY:
+    case Type::LARGE_STRING:
+      return Generator<LargeBinaryType>::Exec;
+    default:
+      DCHECK(false);
+      return ExecFail;
+  }
+}
+
 // Generate a kernel given a templated functor for base binary types. Generates
 // a single kernel for binary/string and large binary / large string. If your
 // kernel implementation needs access to the specific type at compile time,
diff --git a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
index e856c85f610..b4a95f87fac 100644
--- a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
@@ -84,6 +84,52 @@ struct FillNullFunctor<Type, enable_if_t<is_number_type<Type>::value>> {
   }
 };
 
+template <typename Type>
+struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
+  using BuilderType = typename TypeTraits<Type>::BuilderType;
+
+  static void Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) {
+    const ArrayData& input = *batch[0].array();
+    const auto& fill_value_scalar =
+        checked_cast<const BaseBinaryScalar&>(*batch[1].scalar());
+    util::string_view fill_value =
+        static_cast<util::string_view>(*fill_value_scalar.value);
+    ArrayData* output = out->mutable_array();
+
+    // Ensure the kernel is configured properly to have no validity bitmap /
+    // null count 0 unless we explicitly propagate it below.
+    DCHECK(output->buffers[0] == nullptr);
+
+    if (input.MayHaveNulls() && fill_value_scalar.is_valid) {
+      BuilderType builder(input.type, ctx->memory_pool());
+      //
+      KERNEL_RETURN_IF_ERROR(
+          ctx, builder.ReserveData(input.buffers[2]->size() +
+                                   fill_value.length() * input.GetNullCount()));
+      KERNEL_RETURN_IF_ERROR(ctx, builder.Resize(input.length + 1));
+
+      KERNEL_RETURN_IF_ERROR(ctx, VisitArrayDataInline<Type>(
+                                      input,
+                                      [&](util::string_view s) {
+                                        builder.UnsafeAppend(s);
+                                        return Status::OK();
+                                      },
+                                      [&]() {
+                                        builder.UnsafeAppend(fill_value);
+                                        return Status::OK();
+                                      }));
+      std::shared_ptr<Array> string_array;
+      KERNEL_RETURN_IF_ERROR(ctx, builder.Finish(&string_array));
+      *output = *string_array->data();
+      // The builder does not match the logical type, due to
+      // GenerateTypeAgnosticVarBinaryBase
+      output->type = input.type;
+    } else {
+      *output = input;
+    }
+  }
+};
+
 template <typename Type>
 struct FillNullFunctor<Type, enable_if_t<is_boolean_type<Type>::value>> {
   static void Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) {
@@ -153,6 +199,15 @@ void AddBasicFillNullKernels(ScalarKernel kernel, ScalarFunction* func) {
   AddKernels({boolean(), null()});
 }
 
+void AddBinaryFillNullKernels(ScalarKernel kernel, ScalarFunction* func) {
+  for (const std::shared_ptr<DataType>& ty : BaseBinaryTypes()) {
+    kernel.signature =
+        KernelSignature::Make({InputType::Array(ty), InputType::Scalar(ty)}, ty);
+    kernel.exec = GenerateTypeAgnosticVarBinaryBase<FillNullFunctor>(*ty);
+    DCHECK_OK(func->AddKernel(kernel));
+  }
+}
+
 const FunctionDoc fill_null_doc{
     "Replace null elements",
     ("`fill_value` must be a scalar of the same type as `values`.\n"
@@ -170,6 +225,7 @@ void RegisterScalarFillNull(FunctionRegistry* registry) {
     auto fill_null =
         std::make_shared<ScalarFunction>("fill_null", Arity::Binary(), &fill_null_doc);
     AddBasicFillNullKernels(fill_null_base, fill_null.get());
+    AddBinaryFillNullKernels(fill_null_base, fill_null.get());
     DCHECK_OK(registry->AddFunction(fill_null));
   }
 }
diff --git a/cpp/src/arrow/compute/kernels/scalar_fill_null_test.cc b/cpp/src/arrow/compute/kernels/scalar_fill_null_test.cc
index 69ee5994419..de1e0802343 100644
--- a/cpp/src/arrow/compute/kernels/scalar_fill_null_test.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_fill_null_test.cc
@@ -154,5 +154,15 @@ TEST_F(TestFillNullKernel, FillNullTimeStamp) {
   CheckFillNull(time64_type, "[2, 1, 6, null]", Datum(scalar2), "[2, 1, 6, 6]");
 }
 
+TEST_F(TestFillNullKernel, FillNullString) {
+  auto type = large_utf8();
+  auto scalar = std::make_shared<LargeStringScalar>("arrow");
+  // no nulls
+  CheckFillNull(type, R"(["foo", "bar"])", Datum(scalar), R"(["foo", "bar"])");
+  // some nulls
+  CheckFillNull(type, R"(["foo", "bar", null])", Datum(scalar),
+                R"(["foo", "bar", "arrow"])");
+}
+
 }  // namespace compute
 }  // namespace arrow
diff --git a/python/pyarrow/tests/test_compute.py b/python/pyarrow/tests/test_compute.py
index c9c2d7ec6d5..400ef0fd4d5 100644
--- a/python/pyarrow/tests/test_compute.py
+++ b/python/pyarrow/tests/test_compute.py
@@ -870,6 +870,17 @@ def test_fill_null():
     expected = pa.array([None, None, None, None])
     assert result.equals(expected)
 
+    arr = pa.array(['a', 'bb', None])
+    result = arr.fill_null('ccc')
+    expected = pa.array(['a', 'bb', 'ccc'])
+    assert result.equals(expected)
+
+    arr = pa.array([b'a', b'bb', None], type=pa.large_binary())
+    result = arr.fill_null('ccc')
+    expected = pa.array([b'a', b'bb', b'ccc'])
+    assert result.equals(expected)
+
+
 
 @pytest.mark.parametrize('arrow_type', numerical_arrow_types)
 def test_fill_null_array(arrow_type):

From 25080efc913bcf6322734cab8b15111c7adbd646 Mon Sep 17 00:00:00 2001
From: "Maarten A. Breddels" <maartenbreddels@gmail.com>
Date: Tue, 10 Nov 2020 19:18:15 +0100
Subject: [PATCH 02/11] just call the ctor

---
 cpp/src/arrow/compute/kernels/scalar_fill_null.cc | 3 +--
 cpp/src/arrow/compute/kernels/scalar_string.cc    | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
index b4a95f87fac..07e8a46e490 100644
--- a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
@@ -92,8 +92,7 @@ struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
     const ArrayData& input = *batch[0].array();
     const auto& fill_value_scalar =
         checked_cast<const BaseBinaryScalar&>(*batch[1].scalar());
-    util::string_view fill_value =
-        static_cast<util::string_view>(*fill_value_scalar.value);
+    util::string_view fill_value(*fill_value_scalar.value);
     ArrayData* output = out->mutable_array();
 
     // Ensure the kernel is configured properly to have no validity bitmap /
diff --git a/cpp/src/arrow/compute/kernels/scalar_string.cc b/cpp/src/arrow/compute/kernels/scalar_string.cc
index 3f83a9f7a41..eff60b80481 100644
--- a/cpp/src/arrow/compute/kernels/scalar_string.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_string.cc
@@ -947,7 +947,7 @@ struct SplitBaseTransform {
       if (input.is_valid) {
         result->is_valid = true;
         BuilderType builder(input.type, ctx->memory_pool());
-        util::string_view s = static_cast<util::string_view>(*input.value);
+        util::string_view s(*input.value);
         KERNEL_RETURN_IF_ERROR(ctx, Split(s, &builder));
         KERNEL_RETURN_IF_ERROR(ctx, builder.Finish(&result->value));
       }

From cd80ca8e76de83b1b2f8abef06e4953e3d734530 Mon Sep 17 00:00:00 2001
From: "Maarten A. Breddels" <maartenbreddels@gmail.com>
Date: Tue, 10 Nov 2020 19:18:34 +0100
Subject: [PATCH 03/11] pytest use large_binary

---
 python/pyarrow/tests/test_compute.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/python/pyarrow/tests/test_compute.py b/python/pyarrow/tests/test_compute.py
index 400ef0fd4d5..981121a3672 100644
--- a/python/pyarrow/tests/test_compute.py
+++ b/python/pyarrow/tests/test_compute.py
@@ -877,11 +877,10 @@ def test_fill_null():
 
     arr = pa.array([b'a', b'bb', None], type=pa.large_binary())
     result = arr.fill_null('ccc')
-    expected = pa.array([b'a', b'bb', b'ccc'])
+    expected = pa.array([b'a', b'bb', b'ccc'], type=pa.large_binary())
     assert result.equals(expected)
 
 
-
 @pytest.mark.parametrize('arrow_type', numerical_arrow_types)
 def test_fill_null_array(arrow_type):
     arr = pa.array([1, 2, None, 4], type=arrow_type)

From 2b39f7e99f4a7f477dcb72b3836330bab95a51e7 Mon Sep 17 00:00:00 2001
From: "Maarten A. Breddels" <maartenbreddels@gmail.com>
Date: Tue, 10 Nov 2020 19:24:15 +0100
Subject: [PATCH 04/11] correct and complete docs

---
 docs/source/cpp/compute.rst | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/docs/source/cpp/compute.rst b/docs/source/cpp/compute.rst
index c9dfa95512d..413bdb5a481 100644
--- a/docs/source/cpp/compute.rst
+++ b/docs/source/cpp/compute.rst
@@ -436,17 +436,17 @@ Structural transforms
 
 .. XXX (this category is a bit of a hodgepodge)
 
-+--------------------------+------------+---------------------------------------+---------------------+---------+
-| Function name            | Arity      | Input types                           | Output type         | Notes   |
-+==========================+============+=======================================+=====================+=========+
-| fill_null                | Binary     | Boolean, Null, Numeric, Temporal      | Boolean             | \(1)    |
-+--------------------------+------------+---------------------------------------+---------------------+---------+
-| is_null                  | Unary      | Any                                   | Boolean             | \(2)    |
-+--------------------------+------------+---------------------------------------+---------------------+---------+
-| is_valid                 | Unary      | Any                                   | Boolean             | \(2)    |
-+--------------------------+------------+---------------------------------------+---------------------+---------+
-| list_value_length        | Unary      | List-like                             | Int32 or Int64      | \(4)    |
-+--------------------------+------------+---------------------------------------+---------------------+---------+
++--------------------------+------------+------------------------------------------------+---------------------+---------+
+| Function name            | Arity      | Input types                                    | Output type         | Notes   |
++==========================+============+================================================+=====================+=========+
+| fill_null                | Binary     | Boolean, Null, Numeric, Temporal, String-like  | Input type 1        | \(1)    |
++--------------------------+------------+------------------------------------------------+---------------------+---------+
+| is_null                  | Unary      | Any                                            | Boolean             | \(2)    |
++--------------------------+------------+------------------------------------------------+---------------------+---------+
+| is_valid                 | Unary      | Any                                            | Boolean             | \(2)    |
++--------------------------+------------+------------------------------------------------+---------------------+---------+
+| list_value_length        | Unary      | List-like                                      | Int32 or Int64      | \(4)    |
++--------------------------+------------+------------------------------------------------+---------------------+---------+
 
 * \(1) First input must be an array, second input a scalar of the same type.
   Output is an array of the same type as the inputs, and with the same values

From 57fc1bf48fe48e31114f3da8efb4e3be4c67faf4 Mon Sep 17 00:00:00 2001
From: "Maarten A. Breddels" <maartenbreddels@gmail.com>
Date: Tue, 10 Nov 2020 20:01:49 +0100
Subject: [PATCH 05/11] +1 not needed

---
 cpp/src/arrow/compute/kernels/scalar_fill_null.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
index 07e8a46e490..57f88699242 100644
--- a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
@@ -105,7 +105,7 @@ struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
       KERNEL_RETURN_IF_ERROR(
           ctx, builder.ReserveData(input.buffers[2]->size() +
                                    fill_value.length() * input.GetNullCount()));
-      KERNEL_RETURN_IF_ERROR(ctx, builder.Resize(input.length + 1));
+      KERNEL_RETURN_IF_ERROR(ctx, builder.Resize(input.length));
 
       KERNEL_RETURN_IF_ERROR(ctx, VisitArrayDataInline<Type>(
                                       input,

From 8022b706b61ece6d5dd3448c894a2f1abdde3d98 Mon Sep 17 00:00:00 2001
From: "Maarten A. Breddels" <maartenbreddels@gmail.com>
Date: Tue, 10 Nov 2020 20:02:06 +0100
Subject: [PATCH 06/11] not needed to set the type

---
 cpp/src/arrow/compute/kernels/scalar_fill_null.cc | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
index 57f88699242..fb26c8fcc7c 100644
--- a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
@@ -120,9 +120,6 @@ struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
       std::shared_ptr<Array> string_array;
       KERNEL_RETURN_IF_ERROR(ctx, builder.Finish(&string_array));
       *output = *string_array->data();
-      // The builder does not match the logical type, due to
-      // GenerateTypeAgnosticVarBinaryBase
-      output->type = input.type;
     } else {
       *output = input;
     }

From 2dd9fd3a0d4262921023c4812294fc15c5f3cbf0 Mon Sep 17 00:00:00 2001
From: "Maarten A. Breddels" <maartenbreddels@gmail.com>
Date: Wed, 11 Nov 2020 09:30:52 +0100
Subject: [PATCH 07/11] revert: not needed to set the type

---
 cpp/src/arrow/compute/kernels/scalar_fill_null.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
index fb26c8fcc7c..57f88699242 100644
--- a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
@@ -120,6 +120,9 @@ struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
       std::shared_ptr<Array> string_array;
       KERNEL_RETURN_IF_ERROR(ctx, builder.Finish(&string_array));
       *output = *string_array->data();
+      // The builder does not match the logical type, due to
+      // GenerateTypeAgnosticVarBinaryBase
+      output->type = input.type;
     } else {
       *output = input;
     }

From a49e26bf782075992bf46eade8d8973e26ba9d37 Mon Sep 17 00:00:00 2001
From: "Maarten A. Breddels" <maartenbreddels@gmail.com>
Date: Wed, 11 Nov 2020 09:33:23 +0100
Subject: [PATCH 08/11] reorder

---
 .../arrow/compute/kernels/scalar_fill_null.cc | 90 +++++++++----------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
index 57f88699242..4a5835dfcef 100644
--- a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
@@ -84,51 +84,6 @@ struct FillNullFunctor<Type, enable_if_t<is_number_type<Type>::value>> {
   }
 };
 
-template <typename Type>
-struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
-  using BuilderType = typename TypeTraits<Type>::BuilderType;
-
-  static void Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) {
-    const ArrayData& input = *batch[0].array();
-    const auto& fill_value_scalar =
-        checked_cast<const BaseBinaryScalar&>(*batch[1].scalar());
-    util::string_view fill_value(*fill_value_scalar.value);
-    ArrayData* output = out->mutable_array();
-
-    // Ensure the kernel is configured properly to have no validity bitmap /
-    // null count 0 unless we explicitly propagate it below.
-    DCHECK(output->buffers[0] == nullptr);
-
-    if (input.MayHaveNulls() && fill_value_scalar.is_valid) {
-      BuilderType builder(input.type, ctx->memory_pool());
-      //
-      KERNEL_RETURN_IF_ERROR(
-          ctx, builder.ReserveData(input.buffers[2]->size() +
-                                   fill_value.length() * input.GetNullCount()));
-      KERNEL_RETURN_IF_ERROR(ctx, builder.Resize(input.length));
-
-      KERNEL_RETURN_IF_ERROR(ctx, VisitArrayDataInline<Type>(
-                                      input,
-                                      [&](util::string_view s) {
-                                        builder.UnsafeAppend(s);
-                                        return Status::OK();
-                                      },
-                                      [&]() {
-                                        builder.UnsafeAppend(fill_value);
-                                        return Status::OK();
-                                      }));
-      std::shared_ptr<Array> string_array;
-      KERNEL_RETURN_IF_ERROR(ctx, builder.Finish(&string_array));
-      *output = *string_array->data();
-      // The builder does not match the logical type, due to
-      // GenerateTypeAgnosticVarBinaryBase
-      output->type = input.type;
-    } else {
-      *output = input;
-    }
-  }
-};
-
 template <typename Type>
 struct FillNullFunctor<Type, enable_if_t<is_boolean_type<Type>::value>> {
   static void Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) {
@@ -198,6 +153,51 @@ void AddBasicFillNullKernels(ScalarKernel kernel, ScalarFunction* func) {
   AddKernels({boolean(), null()});
 }
 
+template <typename Type>
+struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
+  using BuilderType = typename TypeTraits<Type>::BuilderType;
+
+  static void Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) {
+    const ArrayData& input = *batch[0].array();
+    const auto& fill_value_scalar =
+        checked_cast<const BaseBinaryScalar&>(*batch[1].scalar());
+    util::string_view fill_value(*fill_value_scalar.value);
+    ArrayData* output = out->mutable_array();
+
+    // Ensure the kernel is configured properly to have no validity bitmap /
+    // null count 0 unless we explicitly propagate it below.
+    DCHECK(output->buffers[0] == nullptr);
+
+    if (input.MayHaveNulls() && fill_value_scalar.is_valid) {
+      BuilderType builder(input.type, ctx->memory_pool());
+      //
+      KERNEL_RETURN_IF_ERROR(
+          ctx, builder.ReserveData(input.buffers[2]->size() +
+                                   fill_value.length() * input.GetNullCount()));
+      KERNEL_RETURN_IF_ERROR(ctx, builder.Resize(input.length));
+
+      KERNEL_RETURN_IF_ERROR(ctx, VisitArrayDataInline<Type>(
+                                      input,
+                                      [&](util::string_view s) {
+                                        builder.UnsafeAppend(s);
+                                        return Status::OK();
+                                      },
+                                      [&]() {
+                                        builder.UnsafeAppend(fill_value);
+                                        return Status::OK();
+                                      }));
+      std::shared_ptr<Array> string_array;
+      KERNEL_RETURN_IF_ERROR(ctx, builder.Finish(&string_array));
+      *output = *string_array->data();
+      // The builder does not match the logical type, due to
+      // GenerateTypeAgnosticVarBinaryBase
+      output->type = input.type;
+    } else {
+      *output = input;
+    }
+  }
+};
+
 void AddBinaryFillNullKernels(ScalarKernel kernel, ScalarFunction* func) {
   for (const std::shared_ptr<DataType>& ty : BaseBinaryTypes()) {
     kernel.signature =

From 4c8c42751ec75a52bdb4e1639169103c8c6b97d7 Mon Sep 17 00:00:00 2001
From: "Maarten A. Breddels" <maartenbreddels@gmail.com>
Date: Wed, 11 Nov 2020 09:33:46 +0100
Subject: [PATCH 09/11] correct docs

---
 docs/source/cpp/compute.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/source/cpp/compute.rst b/docs/source/cpp/compute.rst
index 413bdb5a481..081461d2d1f 100644
--- a/docs/source/cpp/compute.rst
+++ b/docs/source/cpp/compute.rst
@@ -439,7 +439,7 @@ Structural transforms
 +--------------------------+------------+------------------------------------------------+---------------------+---------+
 | Function name            | Arity      | Input types                                    | Output type         | Notes   |
 +==========================+============+================================================+=====================+=========+
-| fill_null                | Binary     | Boolean, Null, Numeric, Temporal, String-like  | Input type 1        | \(1)    |
+| fill_null                | Binary     | Boolean, Null, Numeric, Temporal, String-like  | Input type          | \(1)    |
 +--------------------------+------------+------------------------------------------------+---------------------+---------+
 | is_null                  | Unary      | Any                                            | Boolean             | \(2)    |
 +--------------------------+------------+------------------------------------------------+---------------------+---------+

From bfabc7b47313cdbf3d9df7d1a6bd9616eb3a7032 Mon Sep 17 00:00:00 2001
From: "Maarten A. Breddels" <maartenbreddels@gmail.com>
Date: Thu, 12 Nov 2020 20:22:33 +0100
Subject: [PATCH 10/11] small fixes

---
 cpp/src/arrow/compute/kernels/codegen_internal.h  | 2 +-
 cpp/src/arrow/compute/kernels/scalar_fill_null.cc | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/codegen_internal.h b/cpp/src/arrow/compute/kernels/codegen_internal.h
index abcb5b9de1d..9051576e103 100644
--- a/cpp/src/arrow/compute/kernels/codegen_internal.h
+++ b/cpp/src/arrow/compute/kernels/codegen_internal.h
@@ -1058,7 +1058,7 @@ ArrayKernelExec GenerateTypeAgnosticPrimitive(detail::GetTypeId get_id) {
   }
 }
 
-/// similar to GenerateTypeAgnosticPrimitive, but for variable types
+// similar to GenerateTypeAgnosticPrimitive, but for variable types
 template <template <typename...> class Generator>
 ArrayKernelExec GenerateTypeAgnosticVarBinaryBase(detail::GetTypeId get_id) {
   switch (get_id.id) {
diff --git a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
index 4a5835dfcef..7e1e8d9793b 100644
--- a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
@@ -170,7 +170,6 @@ struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
 
     if (input.MayHaveNulls() && fill_value_scalar.is_valid) {
       BuilderType builder(input.type, ctx->memory_pool());
-      //
       KERNEL_RETURN_IF_ERROR(
           ctx, builder.ReserveData(input.buffers[2]->size() +
                                    fill_value.length() * input.GetNullCount()));

From 55f40c7fc251fb16f90d597a0ebc035623bfd20f Mon Sep 17 00:00:00 2001
From: Antoine Pitrou <antoine@python.org>
Date: Mon, 16 Nov 2020 15:21:48 +0100
Subject: [PATCH 11/11] Nits

---
 .../arrow/compute/kernels/scalar_fill_null.cc | 43 +++++++++++--------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
index 7e1e8d9793b..9624f88e68f 100644
--- a/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
+++ b/cpp/src/arrow/compute/kernels/scalar_fill_null.cc
@@ -37,6 +37,8 @@ namespace {
 template <typename Type, typename Enable = void>
 struct FillNullFunctor {};
 
+// Numeric inputs
+
 template <typename Type>
 struct FillNullFunctor<Type, enable_if_t<is_number_type<Type>::value>> {
   using T = typename TypeTraits<Type>::CType;
@@ -84,6 +86,8 @@ struct FillNullFunctor<Type, enable_if_t<is_number_type<Type>::value>> {
   }
 };
 
+// Boolean input
+
 template <typename Type>
 struct FillNullFunctor<Type, enable_if_t<is_boolean_type<Type>::value>> {
   static void Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) {
@@ -131,6 +135,8 @@ struct FillNullFunctor<Type, enable_if_t<is_boolean_type<Type>::value>> {
   }
 };
 
+// Null input
+
 template <typename Type>
 struct FillNullFunctor<Type, enable_if_t<is_null_type<Type>::value>> {
   static void Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) {
@@ -139,19 +145,7 @@ struct FillNullFunctor<Type, enable_if_t<is_null_type<Type>::value>> {
   }
 };
 
-void AddBasicFillNullKernels(ScalarKernel kernel, ScalarFunction* func) {
-  auto AddKernels = [&](const std::vector<std::shared_ptr<DataType>>& types) {
-    for (const std::shared_ptr<DataType>& ty : types) {
-      kernel.signature =
-          KernelSignature::Make({InputType::Array(ty), InputType::Scalar(ty)}, ty);
-      kernel.exec = GenerateTypeAgnosticPrimitive<FillNullFunctor>(*ty);
-      DCHECK_OK(func->AddKernel(kernel));
-    }
-  };
-  AddKernels(NumericTypes());
-  AddKernels(TemporalTypes());
-  AddKernels({boolean(), null()});
-}
+// Binary-like input
 
 template <typename Type>
 struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
@@ -168,11 +162,12 @@ struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
     // null count 0 unless we explicitly propagate it below.
     DCHECK(output->buffers[0] == nullptr);
 
-    if (input.MayHaveNulls() && fill_value_scalar.is_valid) {
+    const int64_t null_count = input.GetNullCount();
+
+    if (null_count > 0 && fill_value_scalar.is_valid) {
       BuilderType builder(input.type, ctx->memory_pool());
-      KERNEL_RETURN_IF_ERROR(
-          ctx, builder.ReserveData(input.buffers[2]->size() +
-                                   fill_value.length() * input.GetNullCount()));
+      KERNEL_RETURN_IF_ERROR(ctx, builder.ReserveData(input.buffers[2]->size() +
+                                                      fill_value.length() * null_count));
       KERNEL_RETURN_IF_ERROR(ctx, builder.Resize(input.length));
 
       KERNEL_RETURN_IF_ERROR(ctx, VisitArrayDataInline<Type>(
@@ -197,6 +192,20 @@ struct FillNullFunctor<Type, enable_if_t<is_base_binary_type<Type>::value>> {
   }
 };
 
+void AddBasicFillNullKernels(ScalarKernel kernel, ScalarFunction* func) {
+  auto AddKernels = [&](const std::vector<std::shared_ptr<DataType>>& types) {
+    for (const std::shared_ptr<DataType>& ty : types) {
+      kernel.signature =
+          KernelSignature::Make({InputType::Array(ty), InputType::Scalar(ty)}, ty);
+      kernel.exec = GenerateTypeAgnosticPrimitive<FillNullFunctor>(*ty);
+      DCHECK_OK(func->AddKernel(kernel));
+    }
+  };
+  AddKernels(NumericTypes());
+  AddKernels(TemporalTypes());
+  AddKernels({boolean(), null()});
+}
+
 void AddBinaryFillNullKernels(ScalarKernel kernel, ScalarFunction* func) {
   for (const std::shared_ptr<DataType>& ty : BaseBinaryTypes()) {
     kernel.signature =