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:
- 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.
- 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:
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 (d0f318d), macOS arm64.
Cause, as far as I can tell (line numbers from d0f318d):
- (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
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 (WriteArrowDictionaryincpp/src/parquet/column_writer.cc) can write wrong statistics in two cases:pa.array([...], type=struct<x: dictionary<...>>)gives you forNonerows. The null rows are not counted innull_count, and a dictionary value that only appears under a null row can become the min or max.min/max are written with
is_min_value_exact/is_max_value_exactset to true, and readers act on these statistics:What other readers make of these files:
SELECT count(*) FROM 'a_dict.parquet' WHERE s.x IS NULLSELECT count(s.x) FROM 'a_dict.parquet'SELECT min(c), max(c) FROM 'b.parquet'With the same data written from a plain
stringchild, or withuse_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, getsmax='suspended', and DataFusion returns that formax(status). Withdata_page_version="2.0", a debug build hitscolumn_writer.cc:1089: Check failed: !page_stats.has_null_count || page_stats.null_count == null_counton case (1).Seen with pyarrow 25.0.1 and on current main (d0f318d), macOS arm64.
Cause, as far as I can tell (line numbers from d0f318d):
WriteIndicesChunkcallsupdate_statson the raw indices (column_writer.cc:2047) beforeMaybeReplaceValidity(: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 toUnique(:2002). The dense path computes statistics afterMaybeReplaceValidity.Uniquereturns 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