Re: [PR] GH-45639: [C++][Statistics] Add support for ARROW:average_byte_width:{exac,approximate} [arrow]

[via GitHub]

kou commented on code in PR #46385:
URL: https://github.com/apache/arrow/pull/46385#discussion_r2194351333


##########
cpp/src/arrow/array/statistics_test.cc:
##########
@@ -81,6 +92,14 @@ TEST(TestArrayStatistics, EqualityNonDoulbeValue) {
   statistics2.distinct_count = 2929;
   ASSERT_EQ(statistics1, statistics2);
 
+  // Although this assertion is meaningless without average_byte_width,
+  // the purpose here is simply to check whether is_average_byte_width_exact
+  // is included in the Equality.
+  statistics1.is_average_byte_width_exact = true;
+  ASSERT_NE(statistics1, statistics2);
+  statistics2.is_average_byte_width_exact = true;
+  ASSERT_EQ(statistics1, statistics2);
+

Review Comment:
   Ah, I understand your intention.
   
   I thought that `TestArrayStatisticsEqualityDoubleValue` is for equality 
logic special double values such as NaN and infinity (and approximate equality).
   
   I think that we can use `TestArrayStatistics` for equality of 
`average_byte_width` if we don't use special double values such as NaN and 
infinity.
   
   How about this?
   
   ```diff
   diff --git a/cpp/src/arrow/array/statistics_test.cc 
b/cpp/src/arrow/array/statistics_test.cc
   index 250c4bb437..22355ce434 100644
   --- a/cpp/src/arrow/array/statistics_test.cc
   +++ b/cpp/src/arrow/array/statistics_test.cc
   @@ -65,7 +65,7 @@ TEST(TestArrayStatistics, Max) {
      ASSERT_FALSE(statistics.is_max_exact);
    }
    
   -TEST(TestArrayStatistics, EqualityNonDoulbeValue) {
   +TEST(TestArrayStatistics, Equals) {
      ArrayStatistics statistics1;
      ArrayStatistics statistics2;
    
   @@ -81,6 +81,16 @@ TEST(TestArrayStatistics, EqualityNonDoulbeValue) {
      statistics2.distinct_count = 2929;
      ASSERT_EQ(statistics1, statistics2);
    
   +  statistics1.average_byte_width = 2.9;
   +  ASSERT_NE(statistics1, statistics2);
   +  statistics2.average_byte_width = 2.9;
   +  ASSERT_EQ(statistics1, statistics2);
   +
   +  statistics1.is_average_byte_width_exact = true;
   +  ASSERT_NE(statistics1, statistics2);
   +  statistics2.is_average_byte_width_exact = true;
   +  ASSERT_EQ(statistics1, statistics2);
   +
      statistics1.min = std::string("world");
      ASSERT_NE(statistics1, statistics2);
      statistics2.min = std::string("world");
   ```



-- 
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