fix casting,includes and equal methods with relevant tests

Author: arashandishgar

cpp/src/arrow/array/statistics.cc

@@ -15,12 +15,14 @@
 // specific language governing permissions and limitations
 // under the License.
 
-// This empty .cc file is for embedding not inlined symbols in
-// arrow::ArrayStatistics into libarrow.
-
 #include "arrow/array/statistics.h"
 
+#include <memory>
+#include <type_traits>
+#include <variant>
+
 #include "arrow/scalar.h"
+#include "arrow/type.h"
 
 namespace arrow {
 
@@ -56,4 +58,38 @@ const std::shared_ptr<DataType>& ArrayStatistics::ValueToArrowType(
   } visitor{array_type};
   return std::visit(visitor, value.value());
 }
+namespace {
+bool ValueTypeEquality(const std::optional<ArrayStatistics::ValueType>& first,
+                       const std::optional<ArrayStatistics::ValueType>& second) {
+  if (first == second) {
+    return true;
+  }
+  if (!first || !second) {
+    return false;
+  }
+
+  return std::visit(
+      [](auto& v1, auto& v2) {
+        if constexpr (std::is_same_v<std::decay_t<decltype(v1)>,
+                                     std::shared_ptr<Scalar>> &&
+                      std::is_same_v<std::decay_t<decltype(v2)>,
+                                     std::shared_ptr<Scalar>>) {
+          if (!v1 || !v2) {
+            // both null case is handled in std::optional and return true
+            return false;
+          }
+          return v1->Equals(*v2);
+        }
+        return false;
+      },
+      first.value(), second.value());
+}
+}  // namespace
+bool ArrayStatistics::Equals(const ArrayStatistics& other) const {
+  return null_count == other.null_count && distinct_count == other.distinct_count &&
+         is_min_exact == other.is_min_exact && is_max_exact == other.is_max_exact &&
+         ValueTypeEquality(this->max, other.max) &&
+         ValueTypeEquality(this->min, other.min);
+}
+
 }  // namespace arrow

cpp/src/arrow/array/statistics.h

@@ -18,15 +18,16 @@
 #pragma once
 
 #include <cstdint>
+#include <memory>
 #include <optional>
 #include <string>
 #include <variant>
 
-#include "arrow/type.h"
 #include "arrow/util/visibility.h"
 
 namespace arrow {
-
+class DataType;
+struct Scalar;
 /// \class ArrayStatistics
 /// \brief Statistics for an Array
 ///
@@ -100,11 +101,7 @@ struct ARROW_EXPORT ArrayStatistics {
   bool is_max_exact = false;
 
   /// \brief Check two statistics for equality
-  bool Equals(const ArrayStatistics& other) const {
-    return null_count == other.null_count && distinct_count == other.distinct_count &&
-           min == other.min && is_min_exact == other.is_min_exact && max == other.max &&
-           is_max_exact == other.is_max_exact;
-  }
+  bool Equals(const ArrayStatistics& other) const;
 
   /// \brief Check two statistics for equality
   bool operator==(const ArrayStatistics& other) const { return Equals(other); }

cpp/src/arrow/array/statistics_test.cc

@@ -18,6 +18,8 @@
 #include <gtest/gtest.h>
 
 #include "arrow/array/statistics.h"
+#include "arrow/scalar.h"
+#include "arrow/testing/gtest_util.h"
 
 namespace arrow {
 
@@ -91,6 +93,28 @@ TEST(ArrayStatisticsTest, TestEquality) {
   ASSERT_NE(statistics1, statistics2);
   statistics2.max = static_cast<int64_t>(-29);
   ASSERT_EQ(statistics1, statistics2);
+  statistics2.max = static_cast<int64_t>(2);
+  ASSERT_NE(statistics1, statistics2);
+
+  statistics1.max = std::nullopt;
+  statistics2.max = std::nullopt;
+  // check the state of both of them are std::nullopt
+  ASSERT_EQ(statistics1.max, statistics2.max);
+  //  the state of one of them is std::nullopt
+  statistics1.max = std::shared_ptr<Scalar>();
+  ASSERT_NE(statistics1, statistics2);
+  // the state of both of them are nullptr
+  statistics2.max = std::shared_ptr<Scalar>();
+  ASSERT_EQ(statistics1, statistics2);
+  ASSERT_OK_AND_ASSIGN(statistics1.max, MakeScalar(int64(), 5));
+  // the state of one of them is nullptr
+  ASSERT_NE(statistics1, statistics2);
+  // the state of one of them has different type
+  statistics2.max = static_cast<int64_t>(10);
+  ASSERT_NE(statistics1.max, statistics2.max);
+  ASSERT_OK_AND_ASSIGN(statistics2.max, MakeScalar(int64(), 5));
+  // the state of both of them are equal
+  ASSERT_EQ(statistics1, statistics2);
 
   statistics1.is_max_exact = true;
   ASSERT_NE(statistics1, statistics2);

cpp/src/arrow/record_batch.cc

@@ -520,17 +520,17 @@ Status EnumerateStatistics(const RecordBatch& record_batch, OnStatistics on_stat
   RETURN_NOT_OK(on_statistics(statistics));
   statistics.start_new_column = false;
 
-  const auto& schema = ExtractArrayDataAndType(record_batch);
-  auto num_fields = static_cast<int64_t>(schema.size());
+  const auto& array_statistics_and_type_vector = ExtractArrayDataAndType(record_batch);
+  auto num_fields = static_cast<int64_t>(array_statistics_and_type_vector.size());
   for (int64_t nth_column = 0; nth_column < num_fields; ++nth_column) {
-    const auto& type = schema[nth_column].second;
-    auto column_statistics = schema[nth_column].first;
+    const auto& type = array_statistics_and_type_vector[nth_column].second;
+    auto column_statistics = array_statistics_and_type_vector[nth_column].first;
     if (!column_statistics) {
       continue;
     }
 
     statistics.start_new_column = true;
-    statistics.nth_column = nth_column;
+    statistics.nth_column = static_cast<int32_t>(nth_column);
     if (column_statistics->null_count.has_value()) {
       statistics.nth_statistics++;
       statistics.key = ARROW_STATISTICS_KEY_NULL_COUNT_EXACT;

cpp/src/arrow/record_batch_test.cc

@@ -1474,16 +1474,16 @@ TEST_F(TestRecordBatch, MakeStatisticsArrayNestedType) {
   statistics_struct->null_count = 0;
   auto struct_array_data = struct_array->data();
   auto statistics_struct_child_a = std::make_shared<ArrayStatistics>();
-  statistics_struct_child_a->min = 1;
+  statistics_struct_child_a->min = int64_t{1};
   struct_array_data->statistics = statistics_struct;
   struct_array_data->child_data[0]->statistics = statistics_struct_child_a;
   auto array_c = ArrayFromJSON(int64(), R"([11,12,13,14,15])");
   array_c->data()->statistics = std::make_shared<ArrayStatistics>();
-  array_c->data()->statistics->max = 15;
+  array_c->data()->statistics->max = int64_t{15};
   auto array_d = ArrayFromJSON(int64(), R"([16,17,18,19,20])");
   auto nested_child = struct_nested_stat->data()->child_data[0];
   nested_child->statistics = std::make_shared<ArrayStatistics>();
-  nested_child->statistics->max = 5;
+  nested_child->statistics->max = int64_t{5};
   nested_child->statistics->is_max_exact = true;
 
   auto rb_schema =
@@ -1492,8 +1492,8 @@ TEST_F(TestRecordBatch, MakeStatisticsArrayNestedType) {
   auto rb = RecordBatch::Make(rb_schema, 5,
                               {struct_array, array_c, array_d, struct_nested_stat});
 
-  auto expected_scalar = std::static_pointer_cast<Scalar>(std::shared_ptr<StructScalar>(
-      new StructScalar({MakeScalar(int64_t{5}), MakeScalar(int64_t{10})}, struct_type)));
+  auto expected_scalar = internal::checked_pointer_cast<StructScalar>(
+      ScalarFromJSON(struct_type, R"([5,10])"));
   auto a = ArrayStatistics::ValueType{std::static_pointer_cast<Scalar>(expected_scalar)};
 
   ASSERT_OK_AND_ASSIGN(
@@ -1536,8 +1536,8 @@ TEST_F(TestRecordBatch, MakeStatisticsArrayNestedNestedType) {
       StructArray::Make({struct_nested_0, struct_nested_1},
                         {field("struct_nested_0", struct_nested_0->type()),
                          field("struct_nested_1", struct_nested_1->type())}));
-  auto expected_scalar = std::static_pointer_cast<Scalar>(std::shared_ptr<StructScalar>(
-      new StructScalar({MakeScalar(int32_t{5}), MakeScalar(int32_t{10})}, struct_type)));
+  auto expected_scalar = internal::checked_pointer_cast<StructScalar>(
+      ScalarFromJSON(struct_type, R"([5,10])"));
   auto rb_schema = schema({field("struct", struct_parent->type())});
   auto rb = RecordBatch::Make(rb_schema, 5, {struct_parent});