NoahKusaba opened a new pull request, #19:
URL: https://github.com/apache/datafusion-iceberg/pull/19
## Which issue does this PR close?
- No issue tracks this.
- Part of the Ballista-Iceberg integration (follows #14), but the bug here
affects any DataFusion user writing to a partitioned table.
## What changes are included in this PR?
An `INSERT` into a **partitioned** table fails when the source has a `NOT
NULL` column where the table's column is optional:
```
Error: Plan("Input schema does not match Iceberg table schema.
Expected schema: Field { "id": nullable Int32 }, Field { "category":
nullable Utf8 }
Input schema: Field { "id": Int32 }, Field { "category": Utf8 }")
```
Every value of a non-nullable column is valid in an optional one, so the
write is safe. The same `INSERT` into an **unpartitioned** table already
succeeds, because `project_with_partition` returns before this check when the
spec is unpartitioned, so today the result depends on whether the table is
partitioned.
`project_with_partition` compared the input and table schemas with `==`. It
now uses Arrow's `Schema::contains`, which allows the input to be narrower in
nullability and is otherwise as strict as before:
- Same columns, same order, same names, same types (checked recursively for
nested types).
- Field metadata is still stripped from both sides before comparing, as
before.
- A nullable input column into a required table column is still rejected, at
any nesting depth.
The public doc of `project_with_partition` now states this contract. The
error message changes from "does not match" to "is not compatible with", since
the schemas no longer need to be equal.
The new unit tests need a partitioned table with a given schema. The three
existing schema-validation tests each built one inline with the same ~50 lines,
so they now share a `table_partitioned_by_id` helper, with their imports moved
to the top of the `tests` module. Their assertions are unchanged apart from the
new error message.
## Are these changes tested?
- `test_insert_not_null_source_into_partitioned_table` (integration):
inserts from a `NOT NULL` `MemTable` into a table partitioned on an optional
column, and reads the rows back. It fails on `main` with the error above.
- `test_schema_validation_nullability`: non-nullable and nullable input into
an optional column are both accepted; into a required column, non-nullable is
accepted and nullable is rejected.
- `test_schema_validation_nested_nullability`: the same four cases for a
field inside a struct.
- The existing matching, mismatched-name and metadata tests still pass.
`cargo fmt --all -- --check`, `cargo clippy --workspace --locked
--all-targets -- -D warnings` and `cargo test --workspace --locked` all pass
locally.
## AI Disclosure
- Used Claude Code to find the bug, write the tests and draft this
description. I reviewed the change and reproduced the failure on `main`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
