You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/07/11 21:29:43 UTC

[GitHub] [arrow] wesm commented on a change in pull request #7635: ARROW-1567: [C++] implement fill null

wesm commented on a change in pull request #7635:
URL: https://github.com/apache/arrow/pull/7635#discussion_r453236952



##########
File path: cpp/src/arrow/compute/kernels/scalar_fill_null.cc
##########
@@ -32,195 +34,117 @@ namespace internal {
 
 namespace {
 
-template <typename OutType, typename InType, typename Enable = void>
+template <typename Type, typename Enable = void>
 struct FillNullFunctor {};
 
-template <typename OutType, typename InType>
-struct FillNullFunctor<OutType, InType, enable_if_t<is_number_type<InType>::value>> {
-  using value_type = typename TypeTraits<InType>::CType;
-  using BuilderType = typename TypeTraits<OutType>::BuilderType;
+template <typename Type>
+struct FillNullFunctor<Type, enable_if_t<is_number_type<Type>::value>> {
+  using T = typename TypeTraits<Type>::CType;
 
   static void Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) {
-    const Datum& in_arr = batch[0];
-    const Datum& fill_value = batch[1];
-
-    if (!in_arr.is_arraylike()) {
-      ctx->SetStatus(Status::Invalid("Values must be Array or ChunkedArray"));
-    }
-    if (!fill_value.is_scalar()) {
-      ctx->SetStatus(Status::Invalid("fill value must be a scalar"));
-    }

Review comment:
       Note: these type checks are not necessary. You can safely assume that once `Exec` is called that the types have already been checked




----------------------------------------------------------------
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:
users@infra.apache.org