winding-lines opened a new issue, #51097:
URL: https://github.com/apache/arrow/issues/51097
### Describe the bug, including details regarding any error messages,
version, and platform.
When a leaf column sits under `struct` under a repeated ancestor (`list`,
`large_list`, `map`) and some of the containers are null or empty, the Parquet
writer undercounts nulls in the column-chunk statistics for **fixed-width**
leaves. The `BYTE_ARRAY` leaf next to it in the same struct is correct, so two
leaves of the same struct disagree about how many of the same 5 level records
are null.
The values themselves round-trip correctly — this is the statistics only.
Reproducer (pyarrow 25.0.1, macOS arm64, also present in the C++ source on
`main`):
```python
import pyarrow as pa
import pyarrow.parquet as pq
typ = pa.list_(pa.struct([("s", pa.string()), ("i32", pa.int32())]))
data = [
[{"s": "a", "i32": 1}], # one element
None, # null list
[], # empty list
[{"s": None, "i32": None}, {"s": "b", "i32": 2}], # two elements, one
all-null
]
tab = pa.table({"col": pa.array(data, type=typ)})
pq.write_table(tab, "x.parquet")
rg = pq.ParquetFile("x.parquet").metadata.row_group(0)
for i in range(rg.num_columns):
c = rg.column(i)
print(c.path_in_schema, c.physical_type, "levels =", c.num_values,
"null_count =", c.statistics.null_count)
print("roundtrip equal:", pq.read_table("x.parquet").equals(tab))
```
Output:
```
col.list.element.s BYTE_ARRAY levels = 5 null_count = 3
col.list.element.i32 INT32 levels = 5 null_count = 1
roundtrip equal: True
```
Both leaves have the same 5 level records: one value, a null list, an empty
list, and two more values of which one is null. Three of the five are null for
both leaves. `s` reports 3, `i32` reports 1 — the null-list and empty-list
slots are counted as present.
`pyarrow`'s `statistics.num_values` is derived as `chunk num_values -
null_count`, so it is wrong in the same way (4 instead of 2). DuckDB 1.5.5's
`parquet_metadata()` reports the same `stats_null_count` values (3 and 1),
which confirms the numbers are what is written to the file, not an artifact of
the pyarrow statistics reader.
The trigger is specifically a fixed-width leaf **under a struct under a
repeated ancestor**. Tested with the same four rows:
| shape | result |
|---|---|
| `list<struct<s: string, i32: int32>>` | `s` correct, `i32` wrong |
| `large_list<struct<...>>` | same |
| `map<string, struct<...>>` | key correct, value `s` correct, value `i32`
wrong |
| `list<int32>` (no struct) | correct |
| `list<list<int32>>` | correct |
| `struct<struct<i32: int32>>` (no repeated ancestor) | correct |
| top-level `struct<s: string, i32: int32>` | correct |
Every fixed-width physical type under `list<struct<...>>` is affected:
`INT32`, `INT64`, `DOUBLE`, `BOOLEAN`, `FIXED_LEN_BYTE_ARRAY`. It also does not
need both cases — a null list alone, or an empty list alone, is enough. It
reproduces with `data_page_version` `1.0` and `2.0`.
### Where it comes from
`MaybeCalculateValidityBits` in `cpp/src/parquet/column_writer.cc` has two
branches. When `bits_buffer_ == nullptr` it computes
```cpp
*null_count = batch_size - *out_values_to_write;
```
over all `batch_size` levels, which is right. When `bits_buffer_ != nullptr`
— which per its own comment is exactly the case where "at least one level of
nullable structs directly precede the leaf node" — it instead takes
`null_count` from `DefLevelsToBitmap`:
```cpp
internal::DefLevelsToBitmap(def_levels, batch_size, level_info_, &io);
*out_values_to_write = io.values_read - io.null_count;
*out_spaced_values_to_write = io.values_read;
*null_count = io.null_count;
```
`io.null_count` counts nulls only among the *spaced* values, i.e. levels
with `def_level >= repeated_ancestor_def_level`. It excludes the empty- and
null-container slots.
That value is then handed to `WriteValuesSpaced` from
`WriteBatchSpacedInternal`, and `WriteValuesSpaced`'s own docstring states the
contract it is meant to satisfy:
```
/// \param num_nulls number of nulls in the values buffer as well as nulls
from the
/// ancestor (e.g. empty lists).
```
The `BYTE_ARRAY` specialisation does not go through that path.
`TypedColumnWriterImpl<ByteArrayType>::WriteArrowDense` recomputes the count
itself against the level count:
```cpp
// Null values in ancestors count as nulls.
const int64_t non_null = data_slice->length() - data_slice->null_count();
...
page_statistics_->IncrementNullCount(batch_size - non_null);
```
which is why the string leaf is right and the fixed-width leaf beside it is
not.
This looks like the residual of PARQUET-2067 / #42980 (fixed by #11281, "Fix
Parquet null count stats for enclosing null lists"). That fix covered the
`bits_buffer_ == nullptr` branch and the `BYTE_ARRAY` path — `list<int32>` is
correct today — but the `bits_buffer_ != nullptr` branch, reached when a
nullable struct sits between the repeated ancestor and the leaf, still uses the
narrower count.
The same `null_count` is also passed to `CommitWriteAndCheckPageLimit`, so
the page-level counts are likely affected too; I did not verify that directly,
as pyarrow does not expose the `DataPageV2` header or `ColumnIndex` null counts.
### Impact
Engines that prune on `null_count` — e.g. deciding a row group cannot
satisfy `IS NULL`, or that a page has no nulls — can make the wrong call on
such a column. Anything that validates a file's statistics against its data
will also see the two leaves of one struct contradict each other.
### Component(s)
C++, Parquet, Python
--
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]
