kita-renji opened a new issue, #51626:
URL: https://github.com/apache/arrow/issues/51626
### Describe the bug, including details regarding any error messages,
version, and platform.
When a `dictionary<*, string|binary>` column is written with dictionary
encoding (the default), the direct dictionary write path
(`WriteArrowDictionary` in `cpp/src/parquet/column_writer.cc`) can write wrong
statistics in two cases:
1. The column sits under a struct that has null rows, and the child slots
under those rows are valid. That's what `pa.array([...], type=struct<x:
dictionary<...>>)` gives you for `None` rows. The null rows are not counted in
`null_count`, and a dictionary value that only appears under a null row can
become the min or max.
2. A write batch contains at least one null and references all but one of
the dictionary entries. The unreferenced entry can become the min or max. A
pandas Categorical with one unused category and a few missing values is enough.
min/max are written with `is_min_value_exact` / `is_max_value_exact` set to
true, and readers act on these statistics:
```python
import pyarrow as pa, pyarrow.parquet as pq
def stats(path):
s = pq.ParquetFile(path).metadata.row_group(0).column(0).statistics
return s.null_count, s.min, s.max
# (1) dictionary leaf under a struct with null rows
d = pa.struct([("x", pa.dictionary(pa.int32(), pa.string()))])
p = pa.struct([("x", pa.string())])
rows = [{"x": "b"}, None, {"x": "c"}, None]
pq.write_table(pa.table({"s": pa.array(rows, type=d)}), "a_dict.parquet")
pq.write_table(pa.table({"s": pa.array(rows, type=p)}), "a_plain.parquet")
print(stats("a_dict.parquet")) # (0, 'b', 'c') expected (2, 'b', 'c')
print(stats("a_plain.parquet")) # (2, 'b', 'c')
x = pa.DictionaryArray.from_arrays(pa.array([0, 1, 0, 1], pa.int32()),
pa.array(["b", "zzz"]))
s = pa.StructArray.from_arrays([x], ["x"], mask=pa.array([False, True,
False, True]))
pq.write_table(pa.table({"s": s}), "a_hidden.parquet")
print(stats("a_hidden.parquet")) # (0, 'b', 'zzz') expected (2, 'b', 'b')
# (2) one unreferenced dictionary entry plus a null
c = pa.DictionaryArray.from_arrays(pa.array([0, None, 0], pa.int32()),
pa.array(["b", "zzz"]))
pq.write_table(pa.table({"c": c}), "b.parquet")
print(stats("b.parquet")) # (1, 'b', 'zzz') expected (1, 'b', 'b')
```
What other readers make of these files:
| Query | File | Result | Expected |
|---|---|---|---|
| DuckDB 1.5.5 `SELECT count(*) FROM 'a_dict.parquet' WHERE s.x IS NULL` |
(1) | 0 | 2 |
| DuckDB 1.5.5 `SELECT count(s.x) FROM 'a_dict.parquet'` | (1) | 4 | 2 |
| DataFusion 55.1 `SELECT min(c), max(c) FROM 'b.parquet'` | (2) | b, zzz |
b, b |
With the same data written from a plain `string` child, or with
`use_dictionary=False`, all of these are correct. A 100k-row pandas Categorical
with categories `["active", "inactive", "suspended"]`, where "suspended" never
occurs and about 1% of rows are missing, gets `max='suspended'`, and DataFusion
returns that for `max(status)`. With `data_page_version="2.0"`, a debug build
hits `column_writer.cc:1089: Check failed: !page_stats.has_null_count ||
page_stats.null_count == null_count` on case (1).
Seen with pyarrow 25.0.1 and on current main (d0f318d1d), macOS arm64.
Cause, as far as I can tell (line numbers from d0f318d1d):
- (1) `WriteIndicesChunk` calls `update_stats` on the raw indices
(column_writer.cc:2047) before `MaybeReplaceValidity` (:2051) replaces their
validity with the one derived from the def levels. So an index under a null
parent is counted as a value (:2017) and passed to `Unique` (:2002). The dense
path computes statistics after `MaybeReplaceValidity`.
- (2) `Unique` returns one null entry when the batch has null indices, so
the "re-use the whole dictionary" check at :2006 passes when exactly one entry
is unreferenced.
Once (1) is fixed, the null parents become null indices, so (1) then runs
into (2). Both need fixing together.
Related: #51097 / #51357 fixed the same null_count invariant for leaves
under a repeated ancestor. Its test has no null struct rows, so it doesn't
reach this path.
I'll open a PR with a fix and tests.
### Component(s)
C++, Parquet
--
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]
