Repository navigation
Internal error: CAST to an extension-type field over a non-null column, under an aggregate (logical vs physical nullability) #26469
Description
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
Castkeeps the input's nullability (cast_output_fieldindatafusion/expr/src/expr_schema.rs). - The physical
CastExpr::return_fieldtakes 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:
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 correct2.
Tested on main at 5b5edc7.
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_fieldkeeps the input's; - physical
CastExpr::return_fielduses the explicit target's; - physical
CastExpr::nullableusesinput || target(Make Physical CastExpr Field-aware and unify cast semantics across physical expressions #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 withsafe: 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.
Plan. Two PRs:
- The fix above, in both layers.
- A follow-up PR that replaces the two copies of the rule with one shared
cast_output_field, called by both layers, so they can't drift again. It doesn't change behavior. This takes the approach from fix: make a cast target's metadata authoritative #24833 (closed in favour of Align metadata propagation through Physical and Logical casts #23169) as a new PR, and puts the function indatafusion/expr-common/src/casts.rs, which already holds cast logic shared between the layers.
Activity