ARROW-47: [C++] Preliminary arrow::Scalar object model

This is the start of a Scalar object model suitable for static and dynamic dispatch to correspond with the existing array and array builder types.

I modified the first aggregation kernel (sum) to use these types for outputs.

Author: Wes McKinney <wesm+git@apache.org>

Closes #3604 from wesm/ARROW-47 and squashes the following commits:

0d01bb3 <Wes McKinney> Fix unit test on MSVC for small integer types
d57f7aa <Wes McKinney> Remove ARROW_GTEST_VENDORED
03ca01c <Wes McKinney> Changes because MSVC tries to instantiate NumericScalar for Time/Timestamp types
271d602 <Wes McKinney> Add notes that Scalar API is experimental
e4a13b4 <Wes McKinney> flake
6626035 <Wes McKinney> Fix up date/time scalars, add tests
704daee <Wes McKinney> Code review comments
d922bd2 <Wes McKinney> Use new Scalar objects in aggregation code
fa89bd0 <Wes McKinney> Drafting, untested
94d5e62 <Wes McKinney> start

Author: wesm

cpp/build-support/run_cpplint.py

@@ -112,6 +112,8 @@ def _check_some_files(completed_processes, filenames):
             if problem_files:
                 msg = "{} had cpplint issues"
                 print("\n".join(map(msg.format, problem_files)))
+                if isinstance(stdout, bytes):
+                    stdout = stdout.decode('utf8')
                 print(stdout, file=sys.stderr)
                 error = True
     except Exception:

cpp/cmake_modules/ThirdpartyToolchain.cmake

@@ -676,6 +676,7 @@ if(ARROW_BUILD_TESTS OR ARROW_BUILD_BENCHMARKS)
     endif()
 
     set(GTEST_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/googletest_ep-prefix/src/googletest_ep")
+    set(GTEST_HOME ${GTEST_PREFIX})
     set(GTEST_INCLUDE_DIR "${GTEST_PREFIX}/include")
     set(GTEST_STATIC_LIB
       "${GTEST_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}gtest${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_STATIC_LIBRARY_SUFFIX}")

cpp/src/arrow/CMakeLists.txt

@@ -90,11 +90,12 @@ set(ARROW_SRCS
     memory_pool.cc
     pretty_print.cc
     record_batch.cc
+    scalar.cc
+    sparse_tensor.cc
     status.cc
     table.cc
     table_builder.cc
     tensor.cc
-    sparse_tensor.cc
     type.cc
     visitor.cc
     csv/converter.cc
@@ -306,6 +307,7 @@ add_arrow_test(buffer-test)
 add_arrow_test(memory_pool-test)
 add_arrow_test(pretty_print-test)
 add_arrow_test(public-api-test)
+add_arrow_test(scalar-test)
 add_arrow_test(status-test)
 add_arrow_test(stl-test)
 add_arrow_test(type-test)

cpp/src/arrow/compute/compute-test.cc

@@ -59,13 +59,16 @@ void CheckImplicitConstructor(enum Datum::type expected_kind) {
 }
 
 TEST(TestDatum, ImplicitConstructors) {
+  CheckImplicitConstructor<Scalar>(Datum::SCALAR);
+
   CheckImplicitConstructor<Array>(Datum::ARRAY);
 
   // Instantiate from array subclass
   CheckImplicitConstructor<BinaryArray>(Datum::ARRAY);
 
   CheckImplicitConstructor<ChunkedArray>(Datum::CHUNKED_ARRAY);
   CheckImplicitConstructor<RecordBatch>(Datum::RECORD_BATCH);
+
   CheckImplicitConstructor<Table>(Datum::TABLE);
 }
 

cpp/src/arrow/compute/kernel.h

@@ -24,6 +24,7 @@
 
 #include "arrow/array.h"
 #include "arrow/record_batch.h"
+#include "arrow/scalar.h"
 #include "arrow/table.h"
 #include "arrow/util/macros.h"
 #include "arrow/util/variant.h"  // IWYU pragma: export
@@ -55,68 +56,20 @@ class ARROW_EXPORT OpKernel {
   virtual ~OpKernel() = default;
 };
 
-/// \brief Placeholder for Scalar values until we implement these
-struct ARROW_EXPORT Scalar {
-  util::variant<bool, uint8_t, int8_t, uint16_t, int16_t, uint32_t, int32_t, uint64_t,
-                int64_t, float, double>
-      value;
-
-  explicit Scalar(bool value) : value(value) {}
-  explicit Scalar(uint8_t value) : value(value) {}
-  explicit Scalar(int8_t value) : value(value) {}
-  explicit Scalar(uint16_t value) : value(value) {}
-  explicit Scalar(int16_t value) : value(value) {}
-  explicit Scalar(uint32_t value) : value(value) {}
-  explicit Scalar(int32_t value) : value(value) {}
-  explicit Scalar(uint64_t value) : value(value) {}
-  explicit Scalar(int64_t value) : value(value) {}
-  explicit Scalar(float value) : value(value) {}
-  explicit Scalar(double value) : value(value) {}
-
-  Type::type kind() const {
-    switch (this->value.which()) {
-      case 0:
-        return Type::BOOL;
-      case 1:
-        return Type::UINT8;
-      case 2:
-        return Type::INT8;
-      case 3:
-        return Type::UINT16;
-      case 4:
-        return Type::INT16;
-      case 5:
-        return Type::UINT32;
-      case 6:
-        return Type::INT32;
-      case 7:
-        return Type::UINT64;
-      case 8:
-        return Type::INT64;
-      case 9:
-        return Type::FLOAT;
-      case 10:
-        return Type::DOUBLE;
-      default:
-        return Type::NA;
-    }
-  }
-};
-
 /// \class Datum
 /// \brief Variant type for various Arrow C++ data structures
 struct ARROW_EXPORT Datum {
   enum type { NONE, SCALAR, ARRAY, CHUNKED_ARRAY, RECORD_BATCH, TABLE, COLLECTION };
 
-  util::variant<decltype(NULLPTR), Scalar, std::shared_ptr<ArrayData>,
+  util::variant<decltype(NULLPTR), std::shared_ptr<Scalar>, std::shared_ptr<ArrayData>,
                 std::shared_ptr<ChunkedArray>, std::shared_ptr<RecordBatch>,
                 std::shared_ptr<Table>, std::vector<Datum>>
       value;
 
   /// \brief Empty datum, to be populated elsewhere
   Datum() : value(NULLPTR) {}
 
-  Datum(const Scalar& value)  // NOLINT implicit conversion
+  Datum(const std::shared_ptr<Scalar>& value)  // NOLINT implicit conversion
       : value(value) {}
   Datum(const std::shared_ptr<ArrayData>& value)  // NOLINT implicit conversion
       : value(value) {}
@@ -188,14 +141,18 @@ struct ARROW_EXPORT Datum {
     return util::get<std::vector<Datum>>(this->value);
   }
 
-  Scalar scalar() const { return util::get<Scalar>(this->value); }
+  std::shared_ptr<Scalar> scalar() const {
+    return util::get<std::shared_ptr<Scalar>>(this->value);
+  }
 
   bool is_array() const { return this->kind() == Datum::ARRAY; }
 
   bool is_arraylike() const {
     return this->kind() == Datum::ARRAY || this->kind() == Datum::CHUNKED_ARRAY;
   }
 
+  bool is_scalar() const { return this->kind() == Datum::SCALAR; }
+
   /// \brief The value type of the variant, if any
   ///
   /// \return nullptr if no type
@@ -204,6 +161,8 @@ struct ARROW_EXPORT Datum {
       return util::get<std::shared_ptr<ArrayData>>(this->value)->type;
     } else if (this->kind() == Datum::CHUNKED_ARRAY) {
       return util::get<std::shared_ptr<ChunkedArray>>(this->value)->type();
+    } else if (this->kind() == Datum::SCALAR) {
+      return util::get<std::shared_ptr<Scalar>>(this->value)->type;
     }
     return NULLPTR;
   }

cpp/src/arrow/compute/kernels/aggregate-test.cc

@@ -25,6 +25,8 @@
 #include "arrow/compute/kernels/sum.h"
 #include "arrow/compute/test-util.h"
 #include "arrow/type.h"
+#include "arrow/type_traits.h"
+#include "arrow/util/checked_cast.h"
 
 #include "arrow/testing/gtest_common.h"
 #include "arrow/testing/gtest_util.h"
@@ -36,47 +38,47 @@ using std::vector;
 namespace arrow {
 namespace compute {
 
-template <typename CType, typename Enable = void>
+template <typename Type, typename Enable = void>
 struct DatumEqual {
   static void EnsureEqual(const Datum& lhs, const Datum& rhs) {}
 };
 
-template <typename CType>
-struct DatumEqual<CType,
-                  typename std::enable_if<std::is_floating_point<CType>::value>::type> {
+template <typename Type>
+struct DatumEqual<Type, typename std::enable_if<IsFloatingPoint<Type>::Value>::type> {
   static constexpr double kArbitraryDoubleErrorBound = 1.0;
+  using ScalarType = typename TypeTraits<Type>::ScalarType;
 
   static void EnsureEqual(const Datum& lhs, const Datum& rhs) {
     ASSERT_EQ(lhs.kind(), rhs.kind());
     if (lhs.kind() == Datum::SCALAR) {
-      ASSERT_EQ(lhs.scalar().kind(), rhs.scalar().kind());
-      ASSERT_NEAR(util::get<CType>(lhs.scalar().value),
-                  util::get<CType>(rhs.scalar().value), kArbitraryDoubleErrorBound);
+      auto left = static_cast<const ScalarType*>(lhs.scalar().get());
+      auto right = static_cast<const ScalarType*>(rhs.scalar().get());
+      ASSERT_EQ(left->type->id(), right->type->id());
+      ASSERT_NEAR(left->value, right->value, kArbitraryDoubleErrorBound);
     }
   }
 };
 
-template <typename CType>
-struct DatumEqual<CType,
-                  typename std::enable_if<!std::is_floating_point<CType>::value>::type> {
+template <typename Type>
+struct DatumEqual<Type, typename std::enable_if<!IsFloatingPoint<Type>::value>::type> {
+  using ScalarType = typename TypeTraits<Type>::ScalarType;
   static void EnsureEqual(const Datum& lhs, const Datum& rhs) {
     ASSERT_EQ(lhs.kind(), rhs.kind());
     if (lhs.kind() == Datum::SCALAR) {
-      ASSERT_EQ(lhs.scalar().kind(), rhs.scalar().kind());
-      ASSERT_EQ(util::get<CType>(lhs.scalar().value),
-                util::get<CType>(rhs.scalar().value));
+      auto left = static_cast<const ScalarType*>(lhs.scalar().get());
+      auto right = static_cast<const ScalarType*>(rhs.scalar().get());
+      ASSERT_EQ(left->type->id(), right->type->id());
+      ASSERT_EQ(left->value, right->value);
     }
   }
 };
 
 template <typename ArrowType>
 void ValidateSum(FunctionContext* ctx, const Array& input, Datum expected) {
-  using CType = typename ArrowType::c_type;
-  using SumType = typename FindAccumulatorType<CType>::Type;
-
+  using OutputType = typename FindAccumulatorType<ArrowType>::Type;
   Datum result;
   ASSERT_OK(Sum(ctx, input, &result));
-  DatumEqual<SumType>::EnsureEqual(result, expected);
+  DatumEqual<OutputType>::EnsureEqual(result, expected);
 }
 
 template <typename ArrowType>
@@ -87,11 +89,11 @@ void ValidateSum(FunctionContext* ctx, const char* json, Datum expected) {
 
 template <typename ArrowType>
 static Datum DummySum(const Array& array) {
-  using CType = typename ArrowType::c_type;
   using ArrayType = typename TypeTraits<ArrowType>::ArrayType;
-  using SumType = typename FindAccumulatorType<CType>::Type;
+  using SumType = typename FindAccumulatorType<ArrowType>::Type;
+  using SumScalarType = typename TypeTraits<SumType>::ScalarType;
 
-  SumType sum = 0;
+  typename SumType::c_type sum = 0;
   int64_t count = 0;
 
   const auto& array_numeric = reinterpret_cast<const ArrayType&>(array);
@@ -104,7 +106,11 @@ static Datum DummySum(const Array& array) {
     }
   }
 
-  return (count > 0) ? Datum(Scalar(sum)) : Datum();
+  if (count > 0) {
+    return Datum(std::make_shared<SumScalarType>(sum));
+  } else {
+    return Datum(std::make_shared<SumScalarType>(0, false));
+  }
 }
 
 template <typename ArrowType>
@@ -115,24 +121,21 @@ void ValidateSum(FunctionContext* ctx, const Array& array) {
 template <typename ArrowType>
 class TestSumKernelNumeric : public ComputeFixture, public TestBase {};
 
-typedef ::testing::Types<Int8Type, UInt8Type, Int16Type, UInt16Type, Int32Type,
-                         UInt32Type, Int64Type, UInt64Type, FloatType, DoubleType>
-    NumericArrowTypes;
-
 TYPED_TEST_CASE(TestSumKernelNumeric, NumericArrowTypes);
 TYPED_TEST(TestSumKernelNumeric, SimpleSum) {
-  using CType = typename TypeParam::c_type;
-  using SumType = typename FindAccumulatorType<CType>::Type;
+  using SumType = typename FindAccumulatorType<TypeParam>::Type;
+  using ScalarType = typename TypeTraits<SumType>::ScalarType;
+  using T = typename TypeParam::c_type;
 
-  ValidateSum<TypeParam>(&this->ctx_, "[]", Datum());
+  ValidateSum<TypeParam>(&this->ctx_, "[]",
+                         Datum(std::make_shared<ScalarType>(0, false)));
 
   ValidateSum<TypeParam>(&this->ctx_, "[0, 1, 2, 3, 4, 5]",
-                         Datum(Scalar(static_cast<SumType>(5 * 6 / 2))));
+                         Datum(std::make_shared<ScalarType>(static_cast<T>(5 * 6 / 2))));
 
-  // Avoid this tests for (U)Int8Type
-  if (sizeof(CType) > 1)
-    ValidateSum<TypeParam>(&this->ctx_, "[1000, null, 300, null, 30, null, 7]",
-                           Datum(Scalar(static_cast<SumType>(1337))));
+  const T expected_result = static_cast<T>(14);
+  ValidateSum<TypeParam>(&this->ctx_, "[1, null, 3, null, 3, null, 7]",
+                         Datum(std::make_shared<ScalarType>(expected_result)));
 }
 
 template <typename ArrowType>

cpp/src/arrow/compute/kernels/sum.cc

@@ -28,9 +28,10 @@
 namespace arrow {
 namespace compute {
 
-template <typename CType, typename SumType = typename FindAccumulatorType<CType>::Type>
+template <typename ArrowType,
+          typename SumType = typename FindAccumulatorType<ArrowType>::Type>
 struct SumState {
-  using ThisType = SumState<CType, SumType>;
+  using ThisType = SumState<ArrowType, SumType>;
 
   ThisType operator+(const ThisType& rhs) const {
     return ThisType(this->count + rhs.count, this->sum + rhs.sum);
@@ -43,11 +44,16 @@ struct SumState {
     return *this;
   }
 
+  std::shared_ptr<Scalar> AsScalar() const {
+    using ScalarType = typename TypeTraits<SumType>::ScalarType;
+    return std::make_shared<ScalarType>(this->sum);
+  }
+
   size_t count = 0;
-  SumType sum = 0;
+  typename SumType::c_type sum = 0;
 };
 
-template <typename ArrowType, typename StateType = SumState<typename ArrowType::c_type>>
+template <typename ArrowType, typename StateType = SumState<ArrowType>>
 class SumAggregateFunction final : public AggregateFunctionStaticState<StateType> {
   using CType = typename TypeTraits<ArrowType>::CType;
   using ArrayType = typename TypeTraits<ArrowType>::ArrayType;
@@ -71,7 +77,12 @@ class SumAggregateFunction final : public AggregateFunctionStaticState<StateType
   }
 
   Status Finalize(const StateType& src, Datum* output) const override {
-    *output = (src.count > 0) ? Datum(Scalar(src.sum)) : Datum();
+    auto boxed = src.AsScalar();
+    if (src.count == 0) {
+      // TODO(wesm): Currently null, but fix this
+      boxed->is_valid = false;
+    }
+    *output = boxed;
     return Status::OK();
   }
 
@@ -185,7 +196,7 @@ Status Sum(FunctionContext* ctx, const Datum& value, Datum* out) {
 }
 
 Status Sum(FunctionContext* ctx, const Array& array, Datum* out) {
-  return Sum(ctx, Datum(array.data()), out);
+  return Sum(ctx, array.data(), out);
 }
 
 }  // namespace compute

cpp/src/arrow/compute/kernels/sum.h

@@ -22,6 +22,8 @@
 #include <type_traits>
 
 #include "arrow/status.h"
+#include "arrow/type.h"
+#include "arrow/type_traits.h"
 #include "arrow/util/visibility.h"
 
 namespace arrow {
@@ -34,25 +36,22 @@ namespace compute {
 // Find the largest compatible primitive type for a primitive type.
 template <typename I, typename Enable = void>
 struct FindAccumulatorType {
-  using Type = double;
+  using Type = DoubleType;
 };
 
 template <typename I>
-struct FindAccumulatorType<I, typename std::enable_if<std::is_integral<I>::value &&
-                                                      std::is_signed<I>::value>::type> {
-  using Type = int64_t;
+struct FindAccumulatorType<I, typename std::enable_if<IsSignedInt<I>::value>::type> {
+  using Type = Int64Type;
 };
 
 template <typename I>
-struct FindAccumulatorType<I, typename std::enable_if<std::is_integral<I>::value &&
-                                                      std::is_unsigned<I>::value>::type> {
-  using Type = uint64_t;
+struct FindAccumulatorType<I, typename std::enable_if<IsUnsignedInt<I>::value>::type> {
+  using Type = UInt64Type;
 };
 
 template <typename I>
-struct FindAccumulatorType<
-    I, typename std::enable_if<std::is_floating_point<I>::value>::type> {
-  using Type = double;
+struct FindAccumulatorType<I, typename std::enable_if<IsFloatingPoint<I>::value>::type> {
+  using Type = DoubleType;
 };
 
 struct Datum;