debugmiller opened a new issue, #104: URL: https://github.com/apache/datafusion-iceberg/issues/104
A null test on a column that is not a primitive (a struct, list or map) fails the query instead of returning rows: ``` DataFusion error: IcebergError(DataInvalid) caused by Execution error: DataInvalid => Accessor for Field s not found ``` `to_iceberg_predicate` converts `Expr::IsNull` / `Expr::IsNotNull` over any column into a unary Iceberg predicate. The conversion has no schema, so it cannot tell a struct column from a primitive one: https://github.com/apache/datafusion-iceberg/blob/f2058c415cd357c6f47fdd25a4ba58f2116fe7a4/crates/datafusion/src/read/predicate.rs#L130-L147 The scan hands that predicate to iceberg-rust's `TableScanBuilder` and fails if `build` does: https://github.com/apache/datafusion-iceberg/blob/f2058c415cd357c6f47fdd25a4ba58f2116fe7a4/crates/datafusion/src/read/scan.rs#L296-L303 `build` binds the predicate to the snapshot's schema ([scan/mod.rs#L317](https://github.com/apache/iceberg-rust/blob/5b82ab99aeaf50bf4f5b3b776ac9cd6380a5f3e3/crates/iceberg/src/scan/mod.rs#L317)), and `Reference::bind` needs an accessor for the field ([expr/term.rs#L309-L332](https://github.com/apache/iceberg-rust/blob/5b82ab99aeaf50bf4f5b3b776ac9cd6380a5f3e3/crates/iceberg/src/expr/term.rs#L309-L332)). `Schema` builds accessors only for primitive fields, including primitives nested in structs, and never for a struct, list or map field itself ([spec/schema/mod.rs#L193-L246](https://github.com/apache/iceberg-rust/blob/5b82ab99aeaf50bf4f5b3b776ac9cd6380a5f3e3/crates/iceberg/src/spec/schema/mod.rs#L193-L246)). The bind error fails the scan. ### To Reproduce With the sqllogictest harness (a new `df_test/nested_null_predicate_pushdown.slt`): ``` statement ok CREATE TABLE default.default.test_nested_null_table (id INT NOT NULL, s STRUCT<a INT, b VARCHAR>) query I INSERT INTO default.default.test_nested_null_table VALUES (1, named_struct('a', 1, 'b', 'x')), (2, NULL), (3, named_struct('a', NULL, 'b', 'y')) ---- 3 query I SELECT count(*) FROM default.default.test_nested_null_table WHERE s IS NOT NULL ---- 2 ``` The `SELECT` fails with the error above. `WHERE s IS NULL` fails the same way. A test on a field inside the struct (`WHERE s.a IS NULL`) works, because `s.a` reaches the conversion as a `get_field` call, which is not converted ([predicate.rs#L239-L250](https://github.com/apache/datafusion-iceberg/blob/f2058c415cd357c6f47fdd25a4ba58f2116fe7a4/crates/datafusion/src/read/predicate.rs#L239-L250)). The table needs at least one snapshot. On an empty table, iceberg-rust returns the scan before binding the predicate ([scan/mod.rs#L294-L308](https://github.com/apache/iceberg-rust/blob/5b82ab99aeaf50bf4f5b3b776ac9cd6380a5f3e3/crates/iceberg/src/scan/mod.rs#L294-L308)), so the query passes. ### Expected behavior The query returns `2` for `IS NOT NULL`, and row `2` for `IS NULL`. ### Possible fix Pushing the predicate to Iceberg is only an optimization here. The provider reports every filter as `Inexact`, so DataFusion keeps the filter and checks every row the scan returns: https://github.com/apache/datafusion-iceberg/blob/f2058c415cd357c6f47fdd25a4ba58f2116fe7a4/crates/datafusion/src/table_provider/catalog_table_provider.rs#L150-L156 So when Iceberg cannot bind part of the predicate, the scan can leave that part out and still return correct results. `IcebergDataSourceBuilder::build` would split the predicate into its top-level conjuncts, bind each to the schema of the snapshot being scanned, and keep only those that bind. In `WHERE id = 1 AND s IS NOT NULL`, `id = 1` is still pushed down and `s IS NOT NULL` is left to DataFusion. Because the check is on the bind result, it also covers any other converted filter Iceberg rejects, not only null tests. I have this with tests and can open a pull request. Two other possible solutions: - #59 adds a bind check that drops the whole predicate when any part fails to bind (it runs into `isnan` on an integer column). That fixes the failure but also turns off pruning for every other filter on the scan. - iceberg-rust could learn to bind a null test on a struct, list or map field. That needs accessors for non-primitive fields and changes to its evaluators, so it is a larger change, and this crate would still need the check above for anything else that does not bind. -- 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]
