kou commented on code in PR #46422: URL: https://github.com/apache/arrow/pull/46422#discussion_r2118270912
########## cpp/src/arrow/array/statistics.h: ########## @@ -127,18 +128,20 @@ struct ARROW_EXPORT ArrayStatistics { /// \brief Whether the maximum value is exact or not 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; - } - - /// \brief Check two statistics for equality - bool operator==(const ArrayStatistics& other) const { return Equals(other); } - - /// \brief Check two statistics for not equality - bool operator!=(const ArrayStatistics& other) const { return !Equals(other); } Review Comment: Why do we need them? ########## cpp/src/arrow/array/statistics.h: ########## @@ -127,18 +128,20 @@ struct ARROW_EXPORT ArrayStatistics { /// \brief Whether the maximum value is exact or not 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; - } - - /// \brief Check two statistics for equality - bool operator==(const ArrayStatistics& other) const { return Equals(other); } - - /// \brief Check two statistics for not equality - bool operator!=(const ArrayStatistics& other) const { return !Equals(other); } + /// \brief Checks whether this ArrayStatistics instance is equal to another. + /// + /// \param other The \ref ArrayStatistics instance to compare against. + /// + /// \param equal_options Options used to compare double values for equality. + /// + /// \param is_approximate If true, \ref arrow::EqualOptions::atol_ is used + /// for comparing double values. + /// + /// \return True if the two \ref arrow::ArrayStatistics instances are equal; otherwise, + /// false. + bool Equals(const ArrayStatistics& other, + const EqualOptions& equal_options = EqualOptions::Defaults(), + bool is_approximate = true) const; Review Comment: Could you add this to `EqualOptions` and remove this from here? See also the discussion: https://github.com/apache/arrow/issues/46436 -- 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. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org