Skip to content

Commit aa9ecc3

Browse files
committed
Refactor cast logic and simplify planner
Moved shared field-aware cast logic to field_aware_cast.rs, removing try_cast.rs dependency on cast.rs for generic behavior. Inlined shared child-lowering step in Cast/TryCast branches by removing the generic planner helper. Added an invariant comment in schema_rewriter.rs to clarify that create_cast_expr expects a column already canonicalized by resolve_physical_column.
1 parent a8d6f7f commit aa9ecc3

6 files changed

Lines changed: 91 additions & 82 deletions

File tree

datafusion/physical-expr-adapter/src/schema_rewriter.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -477,8 +477,10 @@ impl DefaultPhysicalExprAdapterRewriter {
477477
/// Validates type compatibility and creates a field-aware CastExpr if needed.
478478
///
479479
/// Checks whether the physical field can be cast to the logical field type,
480-
/// handling both struct and scalar types. Returns a CastExpr with the
481-
/// appropriate logical target field configuration.
480+
/// handling both struct and scalar types. The `column` passed here is
481+
/// expected to already be canonicalized by [`Self::resolve_physical_column`]
482+
/// so this helper can focus only on compatibility validation and preserving
483+
/// the logical target field configuration.
482484
fn create_cast_expr(
483485
&self,
484486
column: Column,

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

Lines changed: 3 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ use std::fmt;
2020
use std::hash::Hash;
2121
use std::sync::Arc;
2222

23+
use crate::expressions::field_aware_cast::{
24+
resolve_target_field, should_elide_same_type_cast,
25+
};
2326
use crate::physical_expr::PhysicalExpr;
2427

2528
use arrow::compute::{CastOptions, can_cast_types};
@@ -45,46 +48,6 @@ const DEFAULT_SAFE_CAST_OPTIONS: CastOptions<'static> = CastOptions {
4548
format_options: DEFAULT_FORMAT_OPTIONS,
4649
};
4750

48-
pub(crate) fn is_default_target_field(target_field: &FieldRef) -> bool {
49-
target_field.name().is_empty()
50-
&& target_field.is_nullable()
51-
&& target_field.metadata().is_empty()
52-
}
53-
54-
pub(crate) fn resolve_target_field(
55-
expr: &Arc<dyn PhysicalExpr>,
56-
target_field: &FieldRef,
57-
input_schema: &Schema,
58-
force_nullable: bool,
59-
) -> Result<FieldRef> {
60-
if is_default_target_field(target_field) {
61-
let field = expr.return_field(input_schema)?;
62-
let field = field
63-
.as_ref()
64-
.clone()
65-
.with_data_type(target_field.data_type().clone());
66-
let field = if force_nullable {
67-
field.with_nullable(true)
68-
} else {
69-
field
70-
};
71-
Ok(Arc::new(field))
72-
} else {
73-
Ok(Arc::clone(target_field))
74-
}
75-
}
76-
77-
pub(crate) fn should_elide_same_type_cast(
78-
expr: &Arc<dyn PhysicalExpr>,
79-
target_field: &FieldRef,
80-
input_schema: &Schema,
81-
force_nullable: bool,
82-
) -> Result<bool> {
83-
Ok(is_default_target_field(target_field)
84-
|| resolve_target_field(expr, target_field, input_schema, force_nullable)?
85-
== expr.return_field(input_schema)?)
86-
}
87-
8851
/// CAST expression casts an expression to a specific data type and returns a runtime error on invalid cast
8952
#[derive(Debug, Clone, Eq)]
9053
pub struct CastExpr {
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
use std::sync::Arc;
19+
20+
use crate::physical_expr::PhysicalExpr;
21+
use arrow::datatypes::{FieldRef, Schema};
22+
use datafusion_common::Result;
23+
24+
pub(crate) fn is_default_target_field(target_field: &FieldRef) -> bool {
25+
target_field.name().is_empty()
26+
&& target_field.is_nullable()
27+
&& target_field.metadata().is_empty()
28+
}
29+
30+
pub(crate) fn resolve_target_field(
31+
expr: &Arc<dyn PhysicalExpr>,
32+
target_field: &FieldRef,
33+
input_schema: &Schema,
34+
force_nullable: bool,
35+
) -> Result<FieldRef> {
36+
if is_default_target_field(target_field) {
37+
let field = expr.return_field(input_schema)?;
38+
let field = field
39+
.as_ref()
40+
.clone()
41+
.with_data_type(target_field.data_type().clone());
42+
let field = if force_nullable {
43+
field.with_nullable(true)
44+
} else {
45+
field
46+
};
47+
Ok(Arc::new(field))
48+
} else {
49+
Ok(Arc::clone(target_field))
50+
}
51+
}
52+
53+
pub(crate) fn should_elide_same_type_cast(
54+
expr: &Arc<dyn PhysicalExpr>,
55+
target_field: &FieldRef,
56+
input_schema: &Schema,
57+
force_nullable: bool,
58+
) -> Result<bool> {
59+
Ok(is_default_target_field(target_field)
60+
|| resolve_target_field(expr, target_field, input_schema, force_nullable)?
61+
== expr.return_field(input_schema)?)
62+
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ mod cast;
2424
mod cast_column;
2525
mod column;
2626
mod dynamic_filters;
27+
mod field_aware_cast;
2728
mod in_list;
2829
mod is_not_null;
2930
mod is_null;

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@ use std::hash::Hash;
2121
use std::sync::Arc;
2222

2323
use crate::PhysicalExpr;
24-
use crate::expressions::cast::{resolve_target_field, should_elide_same_type_cast};
24+
use crate::expressions::field_aware_cast::{
25+
resolve_target_field, should_elide_same_type_cast,
26+
};
2527
use arrow::compute;
2628
use arrow::compute::CastOptions;
2729
use arrow::datatypes::{DataType, FieldRef, Schema};

datafusion/physical-expr/src/planner.rs

Lines changed: 18 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::{
2323
expressions::{self, Column, Literal, binary, like, similar_to},
2424
};
2525

26-
use arrow::datatypes::{FieldRef, Schema};
26+
use arrow::datatypes::Schema;
2727
use datafusion_common::config::ConfigOptions;
2828
use datafusion_common::metadata::FieldMetadata;
2929
use datafusion_common::{
@@ -288,29 +288,23 @@ pub fn create_physical_expr(
288288
};
289289
Ok(expressions::case(expr, when_then_expr, else_expr)?)
290290
}
291-
Expr::Cast(Cast { expr, field }) => create_physical_cast_like_expr(
292-
expr,
293-
field,
294-
input_dfschema,
295-
input_schema,
296-
execution_props,
297-
|child, input_schema, field| {
298-
expressions::cast_with_target_field_and_options(
299-
child,
300-
input_schema,
301-
field,
302-
None,
303-
)
304-
},
305-
),
306-
Expr::TryCast(TryCast { expr, field }) => create_physical_cast_like_expr(
307-
expr,
308-
field,
309-
input_dfschema,
310-
input_schema,
311-
execution_props,
312-
expressions::try_cast_with_target_field,
313-
),
291+
Expr::Cast(Cast { expr, field }) => {
292+
let child = create_physical_expr(expr, input_dfschema, execution_props)?;
293+
expressions::cast_with_target_field_and_options(
294+
child,
295+
input_schema,
296+
Arc::clone(field),
297+
None,
298+
)
299+
}
300+
Expr::TryCast(TryCast { expr, field }) => {
301+
let child = create_physical_expr(expr, input_dfschema, execution_props)?;
302+
expressions::try_cast_with_target_field(
303+
child,
304+
input_schema,
305+
Arc::clone(field),
306+
)
307+
}
314308
Expr::Not(expr) => {
315309
expressions::not(create_physical_expr(expr, input_dfschema, execution_props)?)
316310
}
@@ -403,21 +397,6 @@ pub fn create_physical_expr(
403397
}
404398
}
405399

406-
fn create_physical_cast_like_expr<F>(
407-
expr: &Expr,
408-
field: &FieldRef,
409-
input_dfschema: &DFSchema,
410-
input_schema: &Schema,
411-
execution_props: &ExecutionProps,
412-
lower: F,
413-
) -> Result<Arc<dyn PhysicalExpr>>
414-
where
415-
F: FnOnce(Arc<dyn PhysicalExpr>, &Schema, FieldRef) -> Result<Arc<dyn PhysicalExpr>>,
416-
{
417-
let child = create_physical_expr(expr, input_dfschema, execution_props)?;
418-
lower(child, input_schema, Arc::clone(field))
419-
}
420-
421400
/// Create vector of Physical Expression from a vector of logical expression
422401
pub fn create_physical_exprs<'a, I>(
423402
exprs: I,

0 commit comments

Comments
 (0)