Remenby31 opened a new issue, #4312:
URL: https://github.com/apache/iceberg-python/issues/4312

   ### Apache Iceberg version
   
   0.12.0 (latest release)
   
   ### Please describe the bug 🐞
   
   Also reproduced on 0.11.1. `main` (e9f7f6b) has the same code path.
   
   When PyIceberg writes a Parquet data file, 
`data_file_statistics_from_parquet_metadata` (`pyiceberg/io/pyarrow.py`) builds 
a column's `lower_bounds` and `upper_bounds` from the row-group statistics. If 
one row group has a statistics object (`is_stats_set` is true, with a null 
count) but no min/max for the column, its values are left out of the bounds. 
The other row groups' bounds are then stored as if they covered the whole file.
   
   PyArrow (25.0.1 here) writes a row group's statistics without min/max when 
one of its values for that column is larger than 4,096 bytes. A string column 
with a single long value is enough. There is no writer option in 
`pyarrow.parquet` to change that.
   
   The stored bounds are then too narrow. `_InclusiveMetricsEvaluator` trusts 
them and drops the file from any scan whose filter value is outside them, so 
`table.scan(row_filter=...)` returns no rows even though the rows are stored. 
The file is not marked as missing statistics: only `is_stats_set == False` and 
`ArrowNotImplementedError` add a column to `invalidate_col` today.
   
   ### Reproduction
   
   Self-contained, no services:
   
   ```python
   import tempfile
   import pyarrow as pa
   import pyarrow.parquet as pq
   from pyiceberg.catalog.sql import SqlCatalog
   from pyiceberg.expressions import EqualTo
   
   warehouse = tempfile.mkdtemp()
   catalog = SqlCatalog("repro", uri=f"sqlite:///{warehouse}/c.db", 
warehouse=f"file://{warehouse}")
   catalog.create_namespace("ns")
   schema = pa.schema([pa.field("k", pa.string())])
   table = catalog.create_table("ns.t", schema=schema, 
properties={"write.parquet.row-group-limit": "10000"})
   long_value = "z" * 5000
   values = [f"a{i:05d}" for i in range(10000)] + [f"b{i:05d}" for i in 
range(9999)] + [long_value]
   table.append(pa.table({"k": values}, schema=schema))
   
   [task] = table.scan().plan_files()
   metadata = 
pq.ParquetFile(task.file.file_path.removeprefix("file://")).metadata
   print([metadata.row_group(i).column(0).statistics.has_min_max for i in 
range(metadata.num_row_groups)])
   print(task.file.lower_bounds.get(1), task.file.upper_bounds.get(1))
   print(table.scan().to_arrow().num_rows)
   print(table.scan(row_filter=EqualTo("k", "b00005")).to_arrow().num_rows)
   print(table.scan(row_filter=EqualTo("k", long_value)).to_arrow().num_rows)
   ```
   
   Output with 0.12.0:
   
   ```
   [True, False]
   b'a00000' b'a09999'
   20000
   0
   0
   ```
   
   Expected: the last two scans return 1 row each. Either the bounds cover 
`b00000` to the long value (truncated per the metrics mode), or the column has 
no bounds for that file.
   
   ### Suggested fix
   
   In `data_file_statistics_from_parquet_metadata`, treat a row group that 
holds non-null values for a column but whose statistics have no min/max like a 
row group without statistics: add the field to `invalidate_col`. A row group 
whose values are all null (`statistics.null_count == column.num_values`) has no 
min/max either and must keep the current behaviour.
   
   ```python
   if stats_col.mode == MetricsMode(MetricModeTypes.COUNTS):
       continue
   
   if not statistics.has_min_max:
       if statistics.has_null_count and statistics.null_count == 
column.num_values:
           continue
       invalidate_col.add(field_id)
       continue
   ```
   
   With this change applied to a copy of 0.12.0, the reproduction prints 
`[True, False]`, `None None`, `20000`, `1`, `1`. A file whose first row group 
holds only nulls (`[None, None]`, then `["b", "c"]`, with 
`write.parquet.row-group-limit` 2) still gets the bounds `b'b'` and `b'c'`.
   
   I can open a pull request with this change and a test built from the 
reproduction above.
   
   ### 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]

Reply via email to