laserninja commented on PR #16110:
URL: https://github.com/apache/iceberg/pull/16110#issuecomment-4954328758
Pushed the `Ignored` change (40188e5c6).
Summary:
- Added the `Ignored` sentinel as sketched, distinct from
`AlwaysTrue`/`AlwaysFalse`.
- `ConvertFilterToParquet` returns `Ignored.INSTANCE` when a predicate
references a column not present in the file schema — both in
`predicate(UnboundPredicate)` (via `findField`/`caseInsensitiveFindField`
returning null) and in `predicate(BoundPredicate)` (when
`schema.idToAlias(fieldId)` is null, which previously produced a null Parquet
column path).
- Boolean composition:
- `not(Ignored)` → `Ignored`
- `and(x, Ignored)` → `x` (dropping a conjunct only widens the scan, which
is safe)
- `or(x, Ignored)` → `Ignored` (the missing column could match, so the OR
can't be pushed)
- `convert()` treats `Ignored` as NOOP for that file
One thing worth flagging on the "TableScan-based test" ask: the modern
reader path (`ReadConf`/`ParquetReader`, used whenever a
`createReaderFunc`/batched func is set) does its row-group filtering with
`ParquetMetricsRowGroupFilter` et al. against the **expected/table schema** and
never calls `ParquetFilters.convert()`. `convert()` is only reached via the
legacy read-support (`ParquetReadBuilder`) path, which binds the filter against
the **physical file schema** — that's the only place the missing-column case
arises. So a `TableScan` going through `createReaderFunc` won't exercise this
code at all.
Given that, I put the coverage in `TestParquetFilters` at the `convert()`
level, which is exactly the per-file boundary where the bug lives — modeling
the schema-evolution case with predicates on a column absent from the file
schema:
- `predicateOnMissingColumnReturnsNoop`
- `notPredicateOnMissingColumnReturnsNoop`
- `orWithMissingColumnReturnsNoop`
- `andWithMissingColumnPushesPresentSide`
If you'd still prefer an end-to-end test through the legacy Avro
read-support path (write a file with the old schema, read with a filter on the
new column), happy to add one. All existing `iceberg-parquet` tests pass.
--
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]
