kita-renji opened a new pull request, #51627: URL: https://github.com/apache/arrow/pull/51627
### Rationale for this change `WriteArrowDictionary` can write wrong Parquet statistics for `dictionary<*, string|binary>` columns (see #51626): 1. Statistics are computed from the leaf indices before their validity is replaced with the one derived from the def levels. An index under a null parent (e.g. a null struct row) can be valid in the leaf array, so it is counted as a value and its dictionary value can become min/max. 2. `Unique()` returns one null entry when a batch has null indices, so the shortcut that re-uses the whole dictionary for min/max fires when exactly one dictionary entry is unreferenced, and that entry can become min/max. min/max are marked exact, and readers use them: DuckDB drops rows for `s.x IS NULL` and miscounts `count(s.x)`, DataFusion returns a value that isn't in the data for `min(c), max(c)`. ### What changes are included in this PR? - Call `MaybeReplaceValidity` before `update_stats` in `WriteIndicesChunk`, so statistics see the same validity as the encoded data. This is the order `WriteArrowDense` already uses. - Don't count the null entry of `Unique()` when checking whether all dictionary entries are referenced. Both are needed: with only the first change, the slots under null parents become null indices and case 2 puts the hidden value back into min/max. ### Are these changes tested? Yes. Two cases added to the `NoNullCountWrittenForRepeatedFields` (PARQUET-2067) suite in `arrow_statistics_test.cc`: a `struct<dictionary<int32, utf8>>` with null rows over valid child slots, and a top-level dictionary with a null and one unreferenced entry. Both fail without the change. All parquet C++ test binaries pass. I also ran a randomized check (not included) that writes dictionary<string/binary> leaves nested in struct/list/large_list/fixed_size_list (up to two levels), with nulls at every level, values hidden under null parents, slices, several batches, small pages and row groups, dictionary fallback and V1/V2 pages, and compares the written statistics with ones computed from the logical values. On main it finds wrong statistics in most nested shapes (and trips the V2 DCHECK); with this change 4500 random cases match. ### Are there any user-facing changes? Files written with dictionary encoding get correct `null_count` and min/max for these columns. No API changes. **This PR contains a "Critical Fix".** It fixes a bug that writes incorrect statistics, which makes other readers return wrong results. The data itself was always written correctly. ### Was AI used for this PR? In accordance to the [AI generation guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code), please disclose below whether and how AI was used in this PR. **PR code and description written by:** - [ ] Human - [x] AI **Reviewed before submission by:** - [x] Human - [ ] AI - [ ] Not reviewed 🤖 Generated with [Claude Code](https://claude.com/claude-code) * GitHub Issue: #51626 -- 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]
