[Statistics][C++] Implement Statistics specification attribute ARROW:distinct_count:approximate

[andishgar]

Describe the bug, including details regarding any error messages, version, and platform.

As I mentioned here, ARROW:distinct_count:approximate has not yet been implemented in C++.

Component(s)

C++

[andishgar]

@kou Regarding this attribute and the other remaining statistics attributes (max_byte_width: {exact, approximate}, null_count: approximate, row_count: approximate), all of them have two forms:

• exact, which accepts an int64_t, and
• approximate, which accepts a double.

Which API design do you suggest?

I’m considering the following options:

1. Implement a new type, such as std::variant<int64_t, double>.
2. Use the existing ValueType.
3. Define two separate variables: one for the exact value and one for the approximate value.

Which approach would you recommend?

[kou]

Good point. Let's use 1.:

• using CountType = std::variant<int64_t, double>; for distinct_count, null_count and row_count
• using SizeType = std::variant<int64_t, double>; for max_byte_width

If we have a good type name that can be used for count value and size value, we can use it instead of separated type name.

[andishgar]

@kou Could we use Boost within Arrow?
I've tried to illustrate below why we need boost through the example code.

#include <iostream>
#include <variant>
#include <string>
#include <boost/variant2/variant.hpp>

namespace {
    using ValueType = std::variant<int64_t, double, std::string>;
    using ValueTypeSubset = std::variant<int64_t, double>;
    // The api of ArrayStatistics will be like this
    // Which I think it's not appropriate
    ValueType ConvertToSuperSet(ValueTypeSubset value) {
        return std::visit([&](auto v) {
            return ValueType(v);
        }, value);
    }
}

int main() {
    ValueType v;
    ValueTypeSubset v1 = int64_t{1};
    // Compile-time error
    /* v = v1; */

    // This is what the ArrayStatistics API might look like,
    // though I don't think it's ideal.
    v = ConvertToSuperSet(v1);
    std::cout << std::get<int64_t>(v) << '\n';

    boost::variant2::variant<int64_t, double, std::string> v2;
    boost::variant2::variant<int64_t, double> v3 = int64_t{1};
    // Using Boost enables this kind of API, which seems cleaner.
    v2 = v3;
    std::cout << boost::variant2::get<int64_t>(v2) << '\n';
}

[kou]

We don't want to use Boost as much as possible... It's difficult to maintain...

    ValueType v;
    ValueTypeSubset v1 = int64_t{1};
    // Compile-time error
    /* v = v1; */

Is this part correct? ValueTypeSubset v1 = v; not ValueTypeSubset v1 = int64_t{1};?

Why do we want to use v1 = v? (I don't understand why we need to convert a subset value to a superset value.)
Do you have more concrete example?

[andishgar]

We need a syntax like this if we want to treat distinct_count as a std::variant<int64_t, double>, and apply the same approach to the remaining statistics schema attributes as well.
However, after giving it some thought, I think it's not a big deal since this is part of the internal implementation.
Sorry for my previous comment — I just made things more complicated than necessary.

[andishgar]

Is this part correct? ValueTypeSubset v1 = v; not ValueTypeSubset v1 = int64_t{1};?

Regarding this, I mean first creating an object of type std::variant<int64_t, double>, and then assigning it to a std::variant<int64_t, double, std::string> object.
Such an assignment is not directly possible with std::variant, but it is allowed with the Boost version.
However, since this kind of conversion would only happen in the internal implementation, I think it's fine to stick with a simple converter function like the one below.

 using ValueType = std::variant<int64_t, double, std::string>;
 using ValueTypeSubset = std::variant<int64_t, double>;

ValueType ConvertToSuperSet(ValueTypeSubset value) {
        return std::visit([&](auto v) {
            return ValueType(v);
        }, value);
    }

[kou]

I see. The following may work too:

diff --git a/cpp/src/arrow/record_batch.cc b/cpp/src/arrow/record_batch.cc
index 04d6890d39..42fd375b0f 100644
--- a/cpp/src/arrow/record_batch.cc
+++ b/cpp/src/arrow/record_batch.cc
@@ -525,7 +525,9 @@ Status EnumerateStatistics(const RecordBatch& record_batch, OnStatistics on_stat
       statistics.nth_statistics++;
       statistics.key = ARROW_STATISTICS_KEY_DISTINCT_COUNT_EXACT;
       statistics.type = int64();
-      statistics.value = column_statistics->distinct_count.value();
+      std::visit([&statistics](auto value) {
+        statistics.value = value;
+      }, column_statistics->distinct_count.value());
       RETURN_NOT_OK(on_statistics(statistics));
       statistics.start_new_column = false;
     }

[andishgar]

Thanks! I actually came up with an even simpler solution in my implementation :).

[kou]

Issue resolved by pull request 47183
#47183