jayzhan211 opened a new issue, #26469:
URL: https://github.com/apache/datafusion/issues/26469
### Describe the bug
When a `CAST` targets an explicit field (one carrying metadata, such as an
extension type from a `TypePlanner`), the logical and physical layers compute
the output nullability differently:
- The logical `Cast` keeps the input's nullability (`cast_output_field` in
`datafusion/expr/src/expr_schema.rs`).
- The physical `CastExpr::return_field` takes the explicit target field's
nullability as-is (`datafusion/physical-expr/src/expressions/cast.rs`).
For a nullable target over a non-null column, the logical field is non-null
and the physical one is nullable. The aggregate input schema check rejects
that, so any aggregate over the cast fails to plan:
```
Internal error: Physical input schema should be the same as the one
converted from logical input schema. Differences:
- field nullability at index 0 [u]: (physical) true vs (logical) false.
```
### To Reproduce
The sqllogictest UUID type planner
(`datafusion/sqllogictest/src/test_context.rs`, used by
`cast_extension_type_metadata.slt`) maps `UUID` to a nullable
`FixedSizeBinary(16)` field with `ARROW:extension:name = arrow.uuid`:
```sql
CREATE TABLE uuid_src AS VALUES
(arrow_cast(X'00010203040506070809000102030506', 'FixedSizeBinary(16)')),
(arrow_cast(X'0f0e0d0c0b0a09080706050403020100', 'FixedSizeBinary(16)'));
-- all three fail with the internal error above
SELECT count(u) FROM (SELECT CAST(column1 AS UUID) AS u FROM uuid_src);
SELECT count(DISTINCT u) FROM (SELECT CAST(column1 AS UUID) AS u FROM
uuid_src);
SELECT count(*) FROM (SELECT CAST(column1 AS UUID) AS u FROM uuid_src) GROUP
BY u;
```
These work:
- the same projection without an aggregate;
- a nullable input column;
- `TRY_CAST` (forced nullable on both sides);
- `SET datafusion.execution.skip_physical_aggregate_schema_check = true`,
which returns the correct `2`.
Tested on `main` at 5b5edc7f3c.
### Expected behavior
`2`, `2`, and two groups of `1`.
### Additional context
#23169 aligned how the two layers propagate cast metadata, but nullability
still follows three rules:
- logical `cast_output_field` keeps the input's;
- physical `CastExpr::return_field` uses the explicit target's;
- physical `CastExpr::nullable` uses `input || target` (#20814).
@paleolimbot noted on #22079 that `nullable()` "frequently disagrees with
`return_field().is_nullable()`". This issue is one place that shows up. The
other direction is a wrong claim: for an explicit non-null target over a
nullable column, `return_field()` reports non-null while the output can contain
NULLs.
**Proposed fix.** Use the physical `nullable()` rule in both layers:
- type-only cast → input nullability;
- explicit target → `input || target`;
- `TryCast`, or a physical cast with `safe: true` → nullable.
This keeps the explicit nullable targets that `UnionExec`/`InterleaveExec`
and recursive CTEs use to widen nullability. I have this locally as a two-line
change plus tests. It fixes the three queries above and passes the full
sqllogictest suite and the core integration tests.
**Trade-off.** An explicit non-null target over a nullable input no longer
makes `return_field()` report non-null. The one in-tree case is the physical
expr adapter, which casts a nullable file column to a non-null table field. Its
scan schema is already built from `nullable()`, so batches don't change; only
direct readers of `return_field()` see a difference. If narrowing is wanted,
@paleolimbot's `NullabilityCast::ForceFalse` idea from #22079 would make it
explicit rather than a side effect of the target field.
**Follow-up.** @adriangb's #24833 proposed a single `cast_output_field`
shared by both layers, so the rule can't drift again.
`datafusion/expr-common/src/casts.rs` already holds cast logic shared between
the layers, so it could live there. Would you both be open to that as a second
PR after the fix? I'm happy to open the fix PR, or to review if one of you
would rather take it.
--
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]
