Subodh-17 opened a new pull request, #26376:
URL: https://github.com/apache/datafusion/pull/26376
# Which issue does this PR close?
Closes #26221.
Closes #3353.
---
# Rationale for this change
When an unaliased SQL query aggregates the same column both with and without
an explicit `CAST`—for example:
```sql
SELECT sum(x), sum(CAST(x AS FLOAT)) FROM (VALUES (1.0), (2.0)) AS t(x);
```
DataFusion previously failed during logical planning with a schema
validation error:
```text
SchemaError: DuplicateUnqualifiedField { name: "sum(CAST(t.x AS Float32))" }
```
(or in earlier versions, stripping all casts during schema name generation
and colliding on `"sum(t.x)"`).
### Root Cause
1. **Implicit vs. Explicit Cast Display Ambiguity:**
During logical query analysis, DataFusion's type coercion framework
automatically injects implicit casts (`Expr::Cast` / `Expr::TryCast`) to
satisfy function signatures (e.g., `sum(x)` on `Int32` receives an implicit
cast to `Int64` or `Float64`). However, `Cast` and `TryCast` previously stored
only `expr: Box<Expr>` and `field: FieldRef`, with no provenance tracking
whether the cast was explicitly written by the user or injected by the
optimizer.
2. **Schema Display Dilemma:**
When formatting schema names for function arguments (`SchemaDisplay`), if
casts were globally preserved, implicit coercion casts appeared in output
headers (e.g., `sum(CAST(t.x AS Int64))`), breaking display conventions and
colliding with explicit casts to the same type. If casts were globally
stripped, `sum(x)` and `sum(CAST(x AS FLOAT))` produced identical names
(`sum(t.x)`), triggering duplicate field errors in `DFSchema::check_names()`.
3. **Scalar UDF Optimizer Invariant Divergence:**
A naive fix that preserves casts across all comma-separated function
argument formatting inadvertently altered `ScalarUDFImpl::schema_name` (e.g.,
`arrow_metadata(CAST(v1 AS DATE), 'k1')`). When `simplify_expressions`
simplified redundant scalar casts and `optimize_projections` merged adjacent
projections, intermediate unaliased expressions dropped the cast wrapper. The
resulting schema name mismatch between pre- and post-optimization plans
violated DataFusion's optimizer invariant assertion (`assert_expected_schema`).
4. **Serialization Provenance Loss:**
Neither Protobuf (`CastNode`, `TryCastNode`) nor Substrait encodes
whether a cast was implicit or explicit. Without provenance preservation and
consumer reconciliation, distributed engines (e.g., Apache Ballista) and
Substrait consumers would reconstruct all casts as explicit, causing schema
discrepancies across round-trips.
---
# What changes are included in this PR?
### 1. Cast Provenance Tracking (`datafusion-expr`)
- Added `pub is_implicit: bool` to `struct Cast` and `struct TryCast`.
- Added explicit constructors:
- `Cast::new_implicit(expr, data_type)`
- `Cast::new_implicit_from_field(expr, field)`
- `TryCast::new_implicit(expr, data_type)`
- `TryCast::new_implicit_from_field(expr, field)`
- Preserved existing constructors (`Cast::new`, `Cast::new_from_field`,
`TryCast::new`, `TryCast::new_from_field`) with `is_implicit = false` for
explicit user casts.
- Updated `type_coercion.rs` and `expr_schema.rs` (`Expr::cast_to`) to
construct implicit casts using `new_implicit`.
- Updated `tree_node.rs` and `order_by.rs` to propagate `is_implicit` during
expression rewrites.
### 2. Semantic Equality, Ordering & Hashing Invariants
- **Intentional Exclusion:** `PartialEq`, `Eq`, `PartialOrd`, and `Hash` on
`Cast` and `TryCast` deliberately compare and hash solely `self.expr` and
`self.field`, ignoring `self.is_implicit`.
- **Rationale:** An explicit cast and an implicit cast targeting the same
data type evaluate to mathematically and computationally identical operations.
Omitting `is_implicit` from equality and hashing ensures that Common
Subexpression Elimination (CSE), distinct grouping rules, and hash sets
continue to deduplicate equivalent expressions without optimizer fragmentation
or execution regressions. Physical execution planners
(`datafusion-physical-expr`) similarly execute kernels identically regardless
of provenance.
### 3. Decoupled Schema Naming for Aggregates vs. Scalars
- **Aggregate Formatting (`UdafSchemaNameBuilder`):**
Introduced internal helper `schema_name_from_aggregate_args` with
`preserve_casts: true`. When formatting aggregate argument schema names:
- Explicit casts (`is_implicit = false`) are preserved: `sum(CAST(t.x AS
Float64))`.
- Implicit casts (`is_implicit = true`) are stripped: `sum(t.x)`.
- Casts of NULL literals (e.g., `CAST(NULL AS VARCHAR)`) continue to
display as `NULL`.
- **Window Aggregate Formatting (`UdafWindowFunctionSchemaNameBuilder`):**
Uses `schema_name_from_function_args` with `preserve_casts: true`,
ensuring window aggregate distinctions are also retained.
- **Scalar UDF Formatting (`ScalarUDFImpl::schema_name`):**
Restored `schema_name_from_exprs_comma_separated_without_space` to
`preserve_casts: false`. Scalar functions adhere to standard SQL display
conventions (stripping casts from unaliased headers), ensuring schema names
remain strictly invariant across algebraic simplifications in
`simplify_expressions` and projection merges in `optimize_projections`.
### 4. Protobuf Wire Compatibility (`datafusion-proto` &
`datafusion-proto-models`)
- Added `optional bool is_implicit = 5;` to `CastNode` and `TryCastNode` in
`datafusion.proto`.
- Regenerated Rust prost bindings (`prost.rs`), Serde codecs (`pbjson.rs`),
and common models (`datafusion_proto_common.rs`).
- `to_proto.rs`: Serializes `is_implicit: Some(cast.is_implicit)`.
- `from_proto.rs`: Deserializes `cast.is_implicit.unwrap_or(false)`. If tag
5 is absent (e.g., from an older version), it safely defaults to `false`.
### 5. Substrait Roundtrip Reconciliation (`datafusion-substrait`)
- In `consumer/utils.rs`, implemented `reconcile_cast_provenance`: when
consumer schema names are renamed against `RelRoot.names`, if an explicit cast
was authored whose target type coincided with the UDF coercion target (e.g.,
`avg(CAST(a AS DOUBLE))`), `reconcile_cast_provenance` checks whether restoring
explicit cast provenance (`is_implicit = false`) yields an exact match with the
target name, avoiding the injection of spurious projection aliases.
- `consumer/expr/function_arguments.rs` and
`consumer/expr/window_function.rs`: Added `mark_implicit_coercion_casts` to
infer implicit coercion casts based on aggregate and window function signatures.
---
# Backward Compatibility
- **Protobuf Wire Compatibility:** Tag 5 is an `optional bool`. Older
decoders encountering tag 5 will safely skip it as an unknown field.
- **Legacy Protobuf Payloads:** When reading messages generated before this
change, `is_implicit` resolves to `None` and safely defaults to `false`.
- **Rust API Compatibility:** `Cast::new` and `Cast::new_from_field` retain
their existing signatures and default to `is_implicit = false`.
- **SQL / Optimizer Display Compatibility:** Top-level projection headers
(e.g., `SELECT CAST(x AS INT)`) and scalar UDFs retain their existing display
names (`x`). Only aggregate function outputs retain explicit cast qualifiers.
---
# Are these changes tested?
Yes. Targeted unit tests, integration tests, and SQL logic tests were
executed and passed cleanly:
1. **SQL Logic Tests (`datafusion-sqllogictest`):**
- `test_files/aggregate.slt`: Verified unaliased `approx_distinct(c9),
approx_distinct(cast(c9 as varchar))` (closing #3353) and added test queries
for `sum(x), sum(CAST(x AS FLOAT))` and `count(x), count(CAST(x AS BIGINT))`.
- `test_files/group_by.slt`: Updated explain plan outputs to reflect
explicit casts in aggregate column headers.
- `test_files/ordered_aggregate_spill.slt` &
`test_files/parquet_statistics.slt`: Updated explain outputs where explicit
casts in aggregate arguments are now properly preserved.
- `test_files/cast_extension_type_metadata.slt`, `metadata.slt`,
`datetime/timestamps.slt`, and `datetime/timestamps_timezone.slt`: Verified
100% pass with zero optimizer invariant failures.
2. **Substrait Integration Tests (`datafusion-substrait`):**
- Added round-trip tests for aggregate and window expressions with
implicit coercion casts, explicit casts matching coercion targets, explicit
casts differing from coercion targets, and explicit `TRY_CAST`.
- Full suite: **234 passed, 0 failed, 6 ignored**.
3. **Protobuf Round-Trip Tests (`datafusion-proto`):**
- Added `roundtrip_aggregate_colliding_risk_cast_plan` verifying
multi-aggregate plans with colliding risks preserve schema names and pass
`check_names()`.
- Added `roundtrip_cast_try_cast_backward_compatibility` validating
legacy payloads omitting tag 5.
- Cast suite: **6 passed, 0 failed**.
4. **SQL Integration Tests (`datafusion-sql`):**
- Added `test_aggregate_with_and_without_cast_on_same_column`.
- Full suite: **605 passed, 0 failed**.
5. **Expression Unit Tests (`datafusion-expr`):**
- Added `test_schema_name_top_level_cast` and
`test_schema_name_function_args_cast`.
- Added `test_udaf_schema_name_with_cast_distinction`.
- All tests passed.
6. **Code Hygiene:**
- `cargo fmt --check`: Passed with exit code 0.
- `git diff --check`: Passed with exit code 0.
---
# Known Limitations
- **Full Sqllogictest Suite Notice:**
When running the full `sqllogictests` suite (532 files) locally without
cloning external git submodules, 523 files pass (98.3%) and 9 files fail
(`describe.slt`, `create_external_table.slt`, `dynamic_file.slt`,
`push_down_filter_regression.slt`, `predicates.slt`, `order.slt`,
`parquet.slt`, `explain.slt`, `intersection.slt`). These 9 failures are
environmental and caused by missing test data in uninitialized
`parquet-testing` and `arrow-testing` git submodules (`No files found at
.../parquet-testing/data/...`). They pass in CI where submodules are
initialized.
---
# Are there any user-facing changes?
Yes. Unaliased SQL queries that aggregate the same column both with and
without an explicit `CAST` will now succeed with distinct column headers
instead of failing with a duplicate field error:
```sql
SELECT sum(x), sum(CAST(x AS FLOAT)) FROM (VALUES (1.0), (2.0)) AS t(x);
-- Output schema column names: "sum(t.x)", "sum(CAST(t.x AS Float32))"
```
--
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]
