andishgar commented on code in PR #46422:
URL: https://github.com/apache/arrow/pull/46422#discussion_r2118297700
##########
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:
It's not possible to set the value of `EqaulOptions` in `== `or `!=`
operations, or alternatively, we could use default value for `EqaulOptions`
in those operations — but I think that would be a bit misleading. It's better
to remove `== `and` != `altogether so that users realize they need to
explicitly configure certain options for comparison
##########
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:
@Kou
If we want to support the `== `operator, we need to implement it like this:
```c++
bool operator==(const ArrayStatistics& other) const {
return Equals(other, EqualOptions::Defaults());
}
```
Should I keep the `==` operator?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
