andishgar commented on code in PR #46385:
URL: https://github.com/apache/arrow/pull/46385#discussion_r2189281143
##########
cpp/src/arrow/array/statistics_test.cc:
##########
@@ -152,4 +163,19 @@ TEST_F(TestArrayStatisticsEqualityDoubleValue,
ApproximateEquals) {
ASSERT_TRUE(statistics1_.Equals(statistics2_,
options_.atol(1e-3).use_atol(true)));
}
+TEST_F(TestArrayStatisticsEqualityDoubleValue, DoubleAttributeCombination) {
+ statistics1_.max = 0.5;
+ ASSERT_FALSE(statistics1_.Equals(statistics2_));
+ statistics2_.max = 0.5;
+ ASSERT_TRUE(statistics1_.Equals(statistics2_));
+ statistics1_.min = 0.5;
+ ASSERT_FALSE(statistics1_.Equals(statistics2_));
+ statistics2_.min = 0.5;
Review Comment:
Actually, this test is intended to cover [this code
path](https://github.com/apache/arrow/blob/be212d7b1162f74992d7fbc31369e5eca33f88a1/cpp/src/arrow/compare.cc#L1567-L1570).
My initial implementation was the code below, but the test revealed that it
was incorrect.
```c++
return left.null_count == right.null_count &&
left.distinct_count == right.distinct_count &&
left.is_min_exact == right.is_min_exact &&
left.is_max_exact == right.is_max_exact &&
left.is_average_byte_width_exact ==
right.is_average_byte_width_exact &&
ArrayStatisticsValueTypeEquals(left.min, right.min, equal_options)
&&
ArrayStatisticsValueTypeEquals(left.max, right.max, equal_options);
ArrayStatisticsValueTypeEquals(left.average_byte_width,
right.average_byte_width,
equal_options) ;
```
Aside from that, since we plan to add double attributes based on the
statistics specification, I suggest keeping this test case to also validate
those additions.
--
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]
