Skip to content

Commit 12acf9b

Browse files
committed
Refactor cast expression handling
Consolidate default-target-field predicate and success construction path in cast_with_target_field to reduce duplicate code in cast.rs. Simplify tests in planner.rs by implementing shared setup helpers and caching return_field() results for standard casts.
1 parent 96a3f32 commit 12acf9b

2 files changed

Lines changed: 38 additions & 48 deletions

File tree

datafusion/physical-expr/src/expressions/cast.rs

Lines changed: 20 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -151,14 +151,8 @@ impl CastExpr {
151151
&self.cast_options
152152
}
153153

154-
fn is_default_target_field(&self) -> bool {
155-
self.target_field.name().is_empty()
156-
&& self.target_field.is_nullable()
157-
&& self.target_field.metadata().is_empty()
158-
}
159-
160154
fn resolved_target_field(&self, input_schema: &Schema) -> Result<FieldRef> {
161-
if self.is_default_target_field() {
155+
if is_default_target_field(&self.target_field) {
162156
self.expr.return_field(input_schema).map(|field| {
163157
Arc::new(
164158
field
@@ -345,36 +339,28 @@ pub fn cast_with_target_field(
345339
if is_default_target_field(&target_field) {
346340
return Ok(Arc::clone(&expr));
347341
}
342+
}
348343

349-
Ok(Arc::new(CastExpr::new_with_target_field(
350-
expr,
351-
target_field,
352-
cast_options,
353-
)))
354-
} else if requires_nested_struct_cast(&expr_type, cast_type) {
355-
if can_cast_named_struct_types(&expr_type, cast_type) {
356-
// Allow casts involving structs (including nested inside Lists, Dictionaries,
357-
// etc.) that pass name-based compatibility validation. This validation is
358-
// applied at planning time (now) to fail fast, rather than deferring errors
359-
// to execution time. The name-based casting logic will be executed at runtime
360-
// via ColumnarValue::cast_to.
361-
Ok(Arc::new(CastExpr::new_with_target_field(
362-
expr,
363-
target_field,
364-
cast_options,
365-
)))
366-
} else {
367-
not_impl_err!("Unsupported CAST from {expr_type} to {cast_type}")
368-
}
369-
} else if can_cast_types(&expr_type, cast_type) {
370-
Ok(Arc::new(CastExpr::new_with_target_field(
371-
expr,
372-
target_field,
373-
cast_options,
374-
)))
344+
let can_build_cast = if requires_nested_struct_cast(&expr_type, cast_type) {
345+
// Allow casts involving structs (including nested inside Lists, Dictionaries,
346+
// etc.) that pass name-based compatibility validation. This validation is
347+
// applied at planning time (now) to fail fast, rather than deferring errors
348+
// to execution time. The name-based casting logic will be executed at runtime
349+
// via ColumnarValue::cast_to.
350+
can_cast_named_struct_types(&expr_type, cast_type)
375351
} else {
376-
not_impl_err!("Unsupported CAST from {expr_type} to {cast_type}")
352+
can_cast_types(&expr_type, cast_type)
353+
};
354+
355+
if !can_build_cast {
356+
return not_impl_err!("Unsupported CAST from {expr_type} to {cast_type}");
377357
}
358+
359+
Ok(Arc::new(CastExpr::new_with_target_field(
360+
expr,
361+
target_field,
362+
cast_options,
363+
)))
378364
}
379365

380366
/// Return a PhysicalExpression representing `expr` casted to

datafusion/physical-expr/src/planner.rs

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,15 @@ mod tests {
439439

440440
use super::*;
441441

442+
fn test_cast_schema() -> Schema {
443+
Schema::new(vec![Field::new("a", DataType::Int32, false)])
444+
}
445+
446+
fn lower_cast_expr(expr: &Expr, schema: &Schema) -> Result<Arc<dyn PhysicalExpr>> {
447+
let df_schema = DFSchema::try_from(schema.clone())?;
448+
create_physical_expr(expr, &df_schema, &ExecutionProps::new())
449+
}
450+
442451
#[test]
443452
fn test_create_physical_expr_scalar_input_output() -> Result<()> {
444453
let expr = col("letter").eq(lit("A"));
@@ -466,8 +475,7 @@ mod tests {
466475

467476
#[test]
468477
fn test_cast_lowering_preserves_target_field_metadata() -> Result<()> {
469-
let schema = Schema::new(vec![Field::new("a", DataType::Int32, false)]);
470-
let df_schema = DFSchema::try_from(schema.clone())?;
478+
let schema = test_cast_schema();
471479
let target_field = Arc::new(
472480
Field::new("cast_target", DataType::Int64, true).with_metadata(
473481
[("target_meta".to_string(), "1".to_string())].into(),
@@ -478,8 +486,7 @@ mod tests {
478486
Arc::clone(&target_field),
479487
));
480488

481-
let physical =
482-
create_physical_expr(&cast_expr, &df_schema, &ExecutionProps::new())?;
489+
let physical = lower_cast_expr(&cast_expr, &schema)?;
483490
let cast = physical
484491
.as_any()
485492
.downcast_ref::<expressions::CastExpr>()
@@ -494,29 +501,27 @@ mod tests {
494501

495502
#[test]
496503
fn test_cast_lowering_preserves_standard_cast_semantics() -> Result<()> {
497-
let schema = Schema::new(vec![Field::new("a", DataType::Int32, false)]);
498-
let df_schema = DFSchema::try_from(schema.clone())?;
504+
let schema = test_cast_schema();
499505
let cast_expr = Expr::Cast(Cast::new(Box::new(col("a")), DataType::Int64));
500506

501-
let physical =
502-
create_physical_expr(&cast_expr, &df_schema, &ExecutionProps::new())?;
507+
let physical = lower_cast_expr(&cast_expr, &schema)?;
503508
let cast = physical
504509
.as_any()
505510
.downcast_ref::<expressions::CastExpr>()
506511
.expect("planner should lower ordinary CAST to CastExpr");
512+
let returned_field = physical.return_field(&schema)?;
507513

508514
assert_eq!(cast.cast_type(), &DataType::Int64);
509-
assert_eq!(physical.return_field(&schema)?.name(), "a");
510-
assert_eq!(physical.return_field(&schema)?.data_type(), &DataType::Int64);
515+
assert_eq!(returned_field.name(), "a");
516+
assert_eq!(returned_field.data_type(), &DataType::Int64);
511517
assert!(!physical.nullable(&schema)?);
512518

513519
Ok(())
514520
}
515521

516522
#[test]
517523
fn test_cast_lowering_preserves_same_type_field_semantics() -> Result<()> {
518-
let schema = Schema::new(vec![Field::new("a", DataType::Int32, false)]);
519-
let df_schema = DFSchema::try_from(schema.clone())?;
524+
let schema = test_cast_schema();
520525
let target_field = Arc::new(
521526
Field::new("same_type_cast", DataType::Int32, true).with_metadata(
522527
[("target_meta".to_string(), "same-type".to_string())].into(),
@@ -527,8 +532,7 @@ mod tests {
527532
Arc::clone(&target_field),
528533
));
529534

530-
let physical =
531-
create_physical_expr(&cast_expr, &df_schema, &ExecutionProps::new())?;
535+
let physical = lower_cast_expr(&cast_expr, &schema)?;
532536
let cast = physical
533537
.as_any()
534538
.downcast_ref::<expressions::CastExpr>()

0 commit comments

Comments
 (0)