slachiewicz opened a new issue, #3833:
URL: https://github.com/apache/iceberg-python/issues/3833
### Apache Iceberg version
main (development)
### Please describe the bug 🐞
A null `list<struct<...>>` is silently rebuilt as an **empty** list. This is
known on
the read path — `tests/integration/test_reads.py::test_null_list_and_map`
currently
asserts the corrupted value, with the correct assertion commented out pending
apache/arrow#38809:
```python
# This should be:
# assert arrow_table["col_list_with_struct"].to_pylist() == [None, [{'test':
1}]]
# Once https://github.com/apache/arrow/issues/38809 has been fixed
assert arrow_table["col_list_with_struct"].to_pylist() == [[], [{"test": 1}]]
```
Two things seem worth reporting on top of that.
**It also affects the write path**, where the consequence is worse. The
Parquet file
pyiceberg writes contains an empty list, so the null is gone at rest and no
reader —
pyiceberg, Spark, Trino — can recover it. On read the file is at least still
correct.
**It does not depend on the upstream Arrow fix.**
`pa.LargeListArray.from_arrays`
takes a `mask` argument — since well before pyiceberg's `pyarrow>=18.0.0`
floor — so
this particular null loss can be fixed independently of apache/arrow#38809,
which is
still open.
This is the `array<struct<>>` case from #251. That issue was closed in March
2025 on
the strength of this test existing, but the assertion it makes is the
corrupted one;
the `array<int>` case in the issue body was genuinely fixed by #252, while
the
`array<struct<test:int>>` case in the issue *title* — which @HonahX flagged
as
remaining broken in
https://github.com/apache/iceberg-python/pull/252#discussion_r1467065763 —
was not.
#### Reproduction (write path)
pyiceberg 0.11.1, pyarrow 25.0.1:
```python
import os, shutil, glob
import pyarrow as pa, pyarrow.parquet as pq
from pyiceberg.catalog.sql import SqlCatalog
WH = "/tmp/wh"; shutil.rmtree(WH, ignore_errors=True); os.makedirs(WH)
sch = pa.schema([
pa.field("id", pa.int32(), nullable=False),
pa.field("l_struct", pa.list_(pa.field("element",
pa.struct([pa.field("x", pa.int32())]), nullable=True)), nullable=True),
pa.field("l_int", pa.list_(pa.field("element", pa.int32(),
nullable=True)), nullable=True),
])
tbl = pa.table({"id": [1, 2, 3, 4],
"l_struct": [[{"x": 1}], [], None, [{"x": 3}]],
"l_int": [[1], [], None, [3]]}, schema=sch)
cat = SqlCatalog("r", uri=f"sqlite:///{WH}/c.db", warehouse=f"file://{WH}")
cat.create_namespace("ns")
it = cat.create_table("ns.t", schema=tbl.schema)
it.append(tbl)
out = it.scan().to_arrow()
for c in ("l_struct", "l_int"):
print(f"{c:9s} in={tbl.column(c).to_pylist()!s:35s}
out={out.column(c).to_pylist()}")
# the loss is already in the file on disk, not in the read path
f = glob.glob(f"{WH}/**/*.parquet", recursive=True)[0]
print("raw parquet:", pq.read_table(f).column("l_struct").to_pylist())
```
Output:
```
l_struct in=[[{'x': 1}], [], None, [{'x': 3}]] out=[[{'x': 1}], [], [],
[{'x': 3}]]
l_int in=[[1], [], None, [3]] out=[[1], [], None, [3]]
raw parquet: [[{'x': 1}], [], [], [{'x': 3}]]
```
`l_int` round-trips correctly, and writing the same `pa.Table` with
`pq.write_table`
preserves the null, so the loss is not pyarrow's.
#### Cause
`ArrowProjectionVisitor.list` rebuilds the array when the element is a struct
(`pyiceberg/io/pyarrow.py:2078` on `main` @ 7539661):
```python
if isinstance(value_array, pa.StructArray):
# This can be removed once this has been fixed:
# https://github.com/apache/arrow/issues/38809
list_array = pa.LargeListArray.from_arrays(list_array.offsets,
value_array)
```
`from_arrays` receives the offsets buffer alone, which cannot express a null
list, and
no mask, so the validity bitmap is dropped. That is also why only this one
shape is
affected: the `struct` visitor passes `mask=struct_array.is_null()`, the
`map` visitor
does not rebuild at all, and a list whose element is a primitive never
enters this
branch. The visitor runs on both paths, which is why the same root cause
shows up as
the read-side assertion above and as the write-side corruption here.
#### Fix
Carrying the mask over is enough:
```python
list_array = pa.LargeListArray.from_arrays(list_array.offsets, value_array,
mask=list_array.is_null())
```
With that change the reproduction above returns `None` for both columns, and
`test_null_list_and_map` passes with its commented-out assertion restored. I
have not
looked at int32-offset or sliced-array handling of this call, which the
existing line
already relies on; that appears independent of the mask.
I have this on a branch with a unit test covering the write path and the
integration
assertion un-commented, and can open a PR.
Found while testing a third-party Iceberg writer against pyiceberg as a
reader.
### Willingness to contribute
- [x] I can contribute a fix for this bug independently
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
