GH-34351: [C++][Parquet] Statistics: add detail documentation and tiny optimization

[mapleFU]

Rationale for this change

What changes are included in this PR?

1. This patch does some tiny optimizations on Parquet C++ Statistics. It does:

For min-max, using std::string. Because assume the case like that:
EncodedStatistics c1;
// do some operations
EncodedStatistics c2 = c1;
c2.set_max("dasdasdassd");
After c2 set, c1 would be set too. So I use std::string here.

2. Force clear ndv count during merging, and set has_distinct_count_ = false, and add some comments
3. Add some specification in Statistics API

Are these changes tested?

Yes

Are there any user-facing changes?

No

• Closes: [C++][Parquet] Statistics Merge ignore setting has flag #34351

[mapleFU]

Some notes:

1. Builder of Statistics will always have has_null_count_, and ignore has_distinct_count_ (not build when building EncodedStatistics). has_min_max_ will be false if page is all nulls or nulls and NaN
2. Reader will able to set distinct_count and has_distinct_count. Which would be valid, but when it call Encoded, the status will not be included in EncodedStatistics

[mapleFU]

Another problem is that:

template <typename DType>
static std::shared_ptr<Statistics> MakeTypedColumnStats(
    const format::ColumnMetaData& metadata, const ColumnDescriptor* descr) {
  // If ColumnOrder is defined, return max_value and min_value
  if (descr->column_order().get_order() == ColumnOrder::TYPE_DEFINED_ORDER) {
    return MakeStatistics<DType>(
        descr, metadata.statistics.min_value, metadata.statistics.max_value,
        metadata.num_values - metadata.statistics.null_count,
        metadata.statistics.null_count, metadata.statistics.distinct_count,
        metadata.statistics.__isset.max_value || metadata.statistics.__isset.min_value,
        metadata.statistics.__isset.null_count,
        metadata.statistics.__isset.distinct_count);
  }
  // Default behavior
  return MakeStatistics<DType>(
      descr, metadata.statistics.min, metadata.statistics.max,
      metadata.num_values - metadata.statistics.null_count,
      metadata.statistics.null_count, metadata.statistics.distinct_count,
      metadata.statistics.__isset.max || metadata.statistics.__isset.min,
      metadata.statistics.__isset.null_count, metadata.statistics.__isset.distinct_count);
}

For the Arrow write file, this is ok, however, when !metadata.statistics.__isset.null_count, the num_values would be wrong, because it's metadata.num_values - metadata.statistics.null_count. Currently I don't know how to fix it.

And when max_rep_level > 1 and has null, I guess num_value would be so confusing...

[mapleFU]

cc @wgtmac @pitrou would you mind take a look? Since Statistics is released, I'm afraid my change will break some invariant...

[mapleFU]

data = [1, 2, 2, None, 4], type = DataType(uint16), physical_type = 'INT32'
min_value = 1, max_value = 4, null_count = 1, num_values = 4, distinct_count = 0

After my reflect, the distinct_count need to be fixed. Can you confirm that @pitrou ?

[pitrou]

@mapleFU How was this data produced?

[pitrou]

(and, yes, distinct_count = 0 is obviously wrong here :-))

[mapleFU]

From here: https://ci.appveyor.com/project/ApacheSoftwareFoundation/arrow/builds/47327471
Let me draft the fix the testing here...

[mapleFU]

#35989 (comment) @pitrou @wgtmac Should we trying to fix this?

when !metadata.statistics.__isset.null_count, the num_values would be wrong

[mapleFU]

[ RUN      ] TwoLevelCacheWithExpirationTest.CleanupPeriodOk
/arrow/cpp/src/parquet/encryption/two_level_cache_with_expiration_test.cc:118: Failure
Expected equality of these values:
  lifetime2->size()
    Which is: 0
  2
[  FAILED  ] TwoLevelCacheWithExpirationTest.CleanupPeriodOk (525 ms)

No idea why this is failed...

[wgtmac]

LGTM. Thanks!

[mapleFU]

Gently ping @pitrou

[pitrou]

LGTM in general, just two minor comments

[pitrou]

Can you open an issue for this? FIXMEs in the code will be forgotten...

[mapleFU]

Created. But actually I found a lot of issues be forgotten #36505

[conbench-apache-arrow]

Conbench analyzed the 6 benchmark runs on commit 60fdc255.

There were 4 benchmark results indicating a performance regression:

• Commit Run on ursa-thinkcentre-m75q at 2023-07-10 11:16:53Z
  • source=cpp-micro, suite=parquet-level-conversion-benchmark
  • source=cpp-micro, suite=parquet-level-conversion-benchmark
• and 2 more (see the report linked below)

The full Conbench report has more details.