andygrove opened a new issue, #57: URL: https://github.com/apache/datafusion-iceberg/issues/57
### Describe the bug `IcebergCatalogTableProvider` caches the table's Arrow schema when it is built ([catalog_table_provider.rs#L81-L84](https://github.com/apache/datafusion-iceberg/blob/72526809d80787c375ebc765ecee9b1db418a232/crates/datafusion/src/table_provider/catalog_table_provider.rs#L81-L84)), and `IcebergCatalogProvider` keeps each provider for its lifetime ([schema.rs#L64-L94](https://github.com/apache/datafusion-iceberg/blob/72526809d80787c375ebc765ecee9b1db418a232/crates/datafusion/src/catalog/schema.rs#L64-L94)). Scans select columns by name from the snapshot they read ([read/scan.rs#L93-L97](https://github.com/apache/datafusion-iceberg/blob/72526809d80787c375ebc765ecee9b1db418a232/crates/datafusion/src/read/scan.rs#L93-L97)). Dropping a column and adding one back under the same name is how a column's type is changed when the change isn't an allowed promotion. After that, a provider built before the change reads the new column but still declares the old column's type and nullability. DataFusion trusts the declared schema: - It folds `price IS NULL` to `false` because the cached `price` is NOT NULL. The plan replaces the scan with `EmptyRelation`, so the count is 0 without reading the table. - Operators that rely on the declared `Float64` panic on the decimal arrays. - `SELECT *` fails because a column declared non-nullable contains nulls. ### To Reproduce 1. Create `ns.t(id INT NOT NULL, price DOUBLE NOT NULL)`, register an `IcebergCatalogProvider` as `c` in session S, and run `INSERT INTO c.ns.t VALUES (1, 1.5)`. 2. Outside S, change `price` to a decimal: drop it with `update_schema().delete_column("price")`, then add it back with `update_schema().add_column(AddColumn::optional("price", Type::Primitive(PrimitiveType::Decimal { precision: 10, scale: 2 })))`. From Spark, that is `ALTER TABLE t DROP COLUMN price` followed by `ALTER TABLE t ADD COLUMN price DECIMAL(10, 2)`. 3. Insert `(2, 2.25)` from a session that registers the catalog after step 2. 4. Run these queries in S and, for comparison, in a new session: | Query | Session S | New session | |---|---|---| | `SELECT count(*) FROM c.ns.t WHERE price IS NULL` | `0` | `1` | | `SELECT count(price) FROM c.ns.t` | panics: `primitive array` | `1` | | `SELECT id, price * 2 FROM c.ns.t` | panics: `primitive array` | `(1, NULL)`, `(2, 4.50)` | | `SELECT * FROM c.ns.t` | error: `Column 'price' is declared as non-nullable but contains null values` | `(1, NULL)`, `(2, 2.25)` | The panics come from arrow-array's `as_primitive` (`cast.rs:849`) and reach the caller as a `Join Error`. If `price` is added back as `DOUBLE` instead, S still counts 0 and still fails `SELECT *`. Only the panics need the type change. ### Expected behavior S returns the same results as the new session, or fails every query on `t` with an error saying that the table's schema changed after the provider was built. It never returns a wrong count or panics. ### Additional context - This is another concrete case of #24 ยง2, on the read path. #56 is the write-path counterpart. - #42 is related but different: there the provider declares the table's current schema while the scan binds to an older snapshot's schema. - #48 lets Ballista rebuild a provider with the schema the client planned against, so a rebuilt provider behaves the same way. - Possible fixes: - in `scan`, check each declared field against the field of the same name in the scanned snapshot's schema (field id from `PARQUET:field_id`, type and nullability), and return an error on a mismatch - resolve the table, and with it the schema, per query (#24) -- 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]
