From dc54a571751f0bc80436eee0487bb3897c76d1d9 Mon Sep 17 00:00:00 2001 From: Siew Kam Onn Date: Thu, 9 Apr 2026 09:53:20 +0800 Subject: [PATCH 1/5] Migrate to direct CastExpr emission in rewriter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update adapter rewriter to emit field-aware CastExpr instead of CastColumnExpr in schema_rewriter.rs. Rename helper from create_cast_column_expr to create_cast_expr. Adjust adapter tests to validate unified CastExpr behavior and maintain target_field() metadata. Modify one test to check nullability via return_field() rather than the old wrapper’s nullability approach. --- .../src/schema_rewriter.rs | 69 +++++++++---------- 1 file changed, 32 insertions(+), 37 deletions(-) diff --git a/datafusion/physical-expr-adapter/src/schema_rewriter.rs b/datafusion/physical-expr-adapter/src/schema_rewriter.rs index 3a255ae05f76f..ca4d325318dcd 100644 --- a/datafusion/physical-expr-adapter/src/schema_rewriter.rs +++ b/datafusion/physical-expr-adapter/src/schema_rewriter.rs @@ -34,11 +34,10 @@ use datafusion_common::{ }; use datafusion_functions::core::getfield::GetFieldFunc; use datafusion_physical_expr::PhysicalExprSimplifier; -use datafusion_physical_expr::expressions::CastColumnExpr; use datafusion_physical_expr::projection::{ProjectionExprs, Projector}; use datafusion_physical_expr::{ ScalarFunctionExpr, - expressions::{self, Column}, + expressions::{self, CastExpr, Column}, }; use datafusion_physical_expr_common::physical_expr::PhysicalExpr; use itertools::Itertools; @@ -439,7 +438,7 @@ impl DefaultPhysicalExprAdapterRewriter { // TODO: add optimization to move the cast from the column to literal expressions in the case of `col = 123` // since that's much cheaper to evalaute. // See https://github.com/apache/datafusion/issues/15780#issuecomment-2824716928 - self.create_cast_column_expr(resolved_column, physical_field, logical_field) + self.create_cast_expr(resolved_column, physical_field, logical_field) } /// Resolves a logical column to the corresponding physical column and field. @@ -475,12 +474,12 @@ impl DefaultPhysicalExprAdapterRewriter { ))) } - /// Validates type compatibility and creates a CastColumnExpr if needed. + /// Validates type compatibility and creates a field-aware CastExpr if needed. /// /// Checks whether the physical field can be cast to the logical field type, - /// handling both struct and scalar types. Returns a CastColumnExpr with the + /// handling both struct and scalar types. Returns a CastExpr with the /// appropriate configuration. - fn create_cast_column_expr( + fn create_cast_expr( &self, column: Column, physical_field: FieldRef, @@ -499,9 +498,8 @@ impl DefaultPhysicalExprAdapterRewriter { logical_field.data_type() )))?; - let cast_expr = Arc::new(CastColumnExpr::new( + let cast_expr = Arc::new(CastExpr::new_with_target_field( Arc::new(column), - physical_field, Arc::new(logical_field.clone()), None, )); @@ -685,7 +683,7 @@ mod tests { let result = adapter.rewrite(column_expr).unwrap(); // Should be wrapped in a cast expression - assert!(result.as_any().downcast_ref::().is_some()); + assert!(result.as_any().downcast_ref::().is_some()); } #[test] @@ -704,8 +702,8 @@ mod tests { let result = adapter.rewrite(Arc::new(Column::new("a", 0)))?; let cast = result .as_any() - .downcast_ref::() - .expect("Expected CastColumnExpr"); + .downcast_ref::() + .expect("Expected CastExpr"); assert_eq!(cast.target_field().data_type(), &DataType::Int64); assert!(!cast.target_field().is_nullable()); @@ -717,8 +715,13 @@ mod tests { Some("1") ); - // Ensure the expression reports the logical nullability regardless of input schema - assert!(!result.nullable(physical_schema.as_ref())?); + // Ensure the expression preserves the logical field nullability/metadata. + let return_field = result.return_field(physical_schema.as_ref())?; + assert!(!return_field.is_nullable()); + assert_eq!( + return_field.metadata().get("logical_meta").map(String::as_str), + Some("1") + ); Ok(()) } @@ -753,9 +756,8 @@ mod tests { println!("Rewritten expression: {result}"); let expected = expressions::BinaryExpr::new( - Arc::new(CastColumnExpr::new( + Arc::new(CastExpr::new_with_target_field( Arc::new(Column::new("a", 0)), - Arc::new(Field::new("a", DataType::Int32, false)), Arc::new(Field::new("a", DataType::Int64, false)), None, )), @@ -841,17 +843,6 @@ mod tests { let result = adapter.rewrite(column_expr).unwrap(); - let physical_struct_fields: Fields = vec![ - Field::new("id", DataType::Int32, false), - Field::new("name", DataType::Utf8, true), - ] - .into(); - let physical_field = Arc::new(Field::new( - "data", - DataType::Struct(physical_struct_fields), - false, - )); - let logical_struct_fields: Fields = vec![ Field::new("id", DataType::Int64, false), Field::new("name", DataType::Utf8View, true), @@ -863,9 +854,8 @@ mod tests { false, )); - let expected = Arc::new(CastColumnExpr::new( + let expected = Arc::new(CastExpr::new_with_target_field( Arc::new(Column::new("data", 0)), - physical_field, logical_field, None, )) as Arc; @@ -1673,11 +1663,11 @@ mod tests { let result = adapter.rewrite(column_expr).unwrap(); - // Should be a CastColumnExpr + // Should be a CastExpr let cast_expr = result .as_any() - .downcast_ref::() - .expect("Expected CastColumnExpr"); + .downcast_ref::() + .expect("Expected CastExpr"); // Verify the inner column points to the correct physical index (1) let inner_col = cast_expr @@ -1696,7 +1686,7 @@ mod tests { } #[test] - fn test_create_cast_column_expr_uses_name_lookup_not_column_index() { + fn test_create_cast_expr_uses_name_lookup_not_column_index() { // Physical schema has column `a` at index 1; index 0 is an incompatible type. let physical_schema = Arc::new(Schema::new(vec![ Field::new("b", DataType::Binary, true), @@ -1716,7 +1706,7 @@ mod tests { // Deliberately provide the wrong index for column `a`. // Regression: this must still resolve against physical field `a` by name. let transformed = rewriter - .create_cast_column_expr( + .create_cast_expr( Column::new("a", 0), Arc::new(physical_schema.field_with_name("a").unwrap().clone()), logical_schema.field_with_name("a").unwrap(), @@ -1726,11 +1716,16 @@ mod tests { let cast_expr = transformed .data .as_any() - .downcast_ref::() - .expect("Expected CastColumnExpr"); + .downcast_ref::() + .expect("Expected CastExpr"); - assert_eq!(cast_expr.input_field().name(), "a"); - assert_eq!(cast_expr.input_field().data_type(), &DataType::Int32); + let inner_col = cast_expr + .expr() + .as_any() + .downcast_ref::() + .expect("Expected inner Column"); + assert_eq!(inner_col.name(), "a"); + assert_eq!(inner_col.index(), 0); assert_eq!(cast_expr.target_field().data_type(), &DataType::Int64); } } From f3d3ec0054c4d3cf4acf7e92897d3016b4b7b33a Mon Sep 17 00:00:00 2001 From: Siew Kam Onn Date: Thu, 9 Apr 2026 10:07:02 +0800 Subject: [PATCH 2/5] Update cast expression handling and tests Change create_cast_expr to accept physical DataType instead of FieldRef to align with the unified CastExpr adapter. Replace the outdated helper-only regression test with test_rewrite_resolves_physical_column_by_name_before_casting. This new test verifies that the name-based resolution still correctly addresses stale column indices prior to building the cast expression. --- .../src/schema_rewriter.rs | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/datafusion/physical-expr-adapter/src/schema_rewriter.rs b/datafusion/physical-expr-adapter/src/schema_rewriter.rs index ca4d325318dcd..43b2c3108b881 100644 --- a/datafusion/physical-expr-adapter/src/schema_rewriter.rs +++ b/datafusion/physical-expr-adapter/src/schema_rewriter.rs @@ -438,7 +438,11 @@ impl DefaultPhysicalExprAdapterRewriter { // TODO: add optimization to move the cast from the column to literal expressions in the case of `col = 123` // since that's much cheaper to evalaute. // See https://github.com/apache/datafusion/issues/15780#issuecomment-2824716928 - self.create_cast_expr(resolved_column, physical_field, logical_field) + self.create_cast_expr( + resolved_column, + physical_field.data_type(), + logical_field, + ) } /// Resolves a logical column to the corresponding physical column and field. @@ -476,25 +480,25 @@ impl DefaultPhysicalExprAdapterRewriter { /// Validates type compatibility and creates a field-aware CastExpr if needed. /// - /// Checks whether the physical field can be cast to the logical field type, + /// Checks whether the physical data type can be cast to the logical field type, /// handling both struct and scalar types. Returns a CastExpr with the /// appropriate configuration. fn create_cast_expr( &self, column: Column, - physical_field: FieldRef, + physical_type: &DataType, logical_field: &Field, ) -> Result>> { validate_data_type_compatibility( column.name(), - physical_field.data_type(), + physical_type, logical_field.data_type(), ) .map_err(|e| DataFusionError::Execution(format!( "Cannot cast column '{}' from '{}' (physical data type) to '{}' (logical data type): {e}", column.name(), - physical_field.data_type(), + physical_type, logical_field.data_type() )))?; @@ -1686,7 +1690,7 @@ mod tests { } #[test] - fn test_create_cast_expr_uses_name_lookup_not_column_index() { + fn test_rewrite_resolves_physical_column_by_name_before_casting() { // Physical schema has column `a` at index 1; index 0 is an incompatible type. let physical_schema = Arc::new(Schema::new(vec![ Field::new("b", DataType::Binary, true), @@ -1698,23 +1702,15 @@ mod tests { Field::new("b", DataType::Binary, true), ])); - let rewriter = DefaultPhysicalExprAdapterRewriter { - logical_file_schema: Arc::clone(&logical_schema), - physical_file_schema: Arc::clone(&physical_schema), - }; + let factory = DefaultPhysicalExprAdapterFactory; + let adapter = factory + .create(Arc::clone(&logical_schema), Arc::clone(&physical_schema)) + .unwrap(); // Deliberately provide the wrong index for column `a`. // Regression: this must still resolve against physical field `a` by name. - let transformed = rewriter - .create_cast_expr( - Column::new("a", 0), - Arc::new(physical_schema.field_with_name("a").unwrap().clone()), - logical_schema.field_with_name("a").unwrap(), - ) - .unwrap(); - - let cast_expr = transformed - .data + let rewritten = adapter.rewrite(Arc::new(Column::new("a", 0))).unwrap(); + let cast_expr = rewritten .as_any() .downcast_ref::() .expect("Expected CastExpr"); @@ -1725,7 +1721,7 @@ mod tests { .downcast_ref::() .expect("Expected inner Column"); assert_eq!(inner_col.name(), "a"); - assert_eq!(inner_col.index(), 0); + assert_eq!(inner_col.index(), 1); assert_eq!(cast_expr.target_field().data_type(), &DataType::Int64); } } From fc934d86dba8d78f4384f93a2dadc3ec3ddf7eb9 Mon Sep 17 00:00:00 2001 From: Siew Kam Onn Date: Thu, 9 Apr 2026 10:16:04 +0800 Subject: [PATCH 3/5] Refactor column handling and tests Inline cast helper in rewrite_column and streamline the column/field matching logic for clarity. Simplify the construction of results in resolve_physical_column. Reduce test duplication with local helpers for CastExpr and inner Column assertions. Utilize a helper adapter factory to reuse stale-index test setup. Simplify metadata/nullability test by asserting the return_field() contract, and replace complex string-based expectations with precise structural assertions. --- .../src/schema_rewriter.rs | 230 ++++++++---------- 1 file changed, 97 insertions(+), 133 deletions(-) diff --git a/datafusion/physical-expr-adapter/src/schema_rewriter.rs b/datafusion/physical-expr-adapter/src/schema_rewriter.rs index 43b2c3108b881..d62a9857469c7 100644 --- a/datafusion/physical-expr-adapter/src/schema_rewriter.rs +++ b/datafusion/physical-expr-adapter/src/schema_rewriter.rs @@ -25,7 +25,7 @@ use std::hash::Hash; use std::sync::Arc; use arrow::array::RecordBatch; -use arrow::datatypes::{DataType, Field, FieldRef, SchemaRef}; +use arrow::datatypes::{DataType, FieldRef, SchemaRef}; use datafusion_common::{ DataFusionError, Result, ScalarValue, exec_err, metadata::FieldMetadata, @@ -422,15 +422,14 @@ impl DefaultPhysicalExprAdapterRewriter { ))); }; - if resolved_column.index() == column.index() - && logical_field == physical_field.as_ref() - { - return Ok(Transformed::no(expr)); - } - - if logical_field == physical_field.as_ref() { - // If the fields match (including metadata/nullability), we can use the column as is - return Ok(Transformed::yes(Arc::new(resolved_column))); + let fields_match = logical_field == physical_field.as_ref(); + match (resolved_column.index() == column.index(), fields_match) { + (true, true) => return Ok(Transformed::no(expr)), + (_, true) => { + // If the fields match (including metadata/nullability), we can use the column as is + return Ok(Transformed::yes(Arc::new(resolved_column))); + } + (_, false) => {} } // We need a cast expression whenever the logical and physical fields differ, @@ -438,11 +437,25 @@ impl DefaultPhysicalExprAdapterRewriter { // TODO: add optimization to move the cast from the column to literal expressions in the case of `col = 123` // since that's much cheaper to evalaute. // See https://github.com/apache/datafusion/issues/15780#issuecomment-2824716928 - self.create_cast_expr( - resolved_column, + validate_data_type_compatibility( + resolved_column.name(), physical_field.data_type(), - logical_field, + logical_field.data_type(), ) + .map_err(|e| { + DataFusionError::Execution(format!( + "Cannot cast column '{}' from '{}' (physical data type) to '{}' (logical data type): {e}", + resolved_column.name(), + physical_field.data_type(), + logical_field.data_type() + )) + })?; + + Ok(Transformed::yes(Arc::new(CastExpr::new_with_target_field( + Arc::new(resolved_column), + Arc::new(logical_field.clone()), + None, + )))) } /// Resolves a logical column to the corresponding physical column and field. @@ -468,47 +481,10 @@ impl DefaultPhysicalExprAdapterRewriter { Column::new_with_schema(column.name(), self.physical_file_schema.as_ref())? }; - Ok(Some(( - column, - Arc::new( - self.physical_file_schema - .field(physical_column_index) - .clone(), - ), - ))) - } - - /// Validates type compatibility and creates a field-aware CastExpr if needed. - /// - /// Checks whether the physical data type can be cast to the logical field type, - /// handling both struct and scalar types. Returns a CastExpr with the - /// appropriate configuration. - fn create_cast_expr( - &self, - column: Column, - physical_type: &DataType, - logical_field: &Field, - ) -> Result>> { - validate_data_type_compatibility( - column.name(), - physical_type, - logical_field.data_type(), - ) - .map_err(|e| - DataFusionError::Execution(format!( - "Cannot cast column '{}' from '{}' (physical data type) to '{}' (logical data type): {e}", - column.name(), - physical_type, - logical_field.data_type() - )))?; - - let cast_expr = Arc::new(CastExpr::new_with_target_field( - Arc::new(column), - Arc::new(logical_field.clone()), - None, - )); + let physical_field = + Arc::new(self.physical_file_schema.field(physical_column_index).clone()); - Ok(Transformed::yes(cast_expr)) + Ok(Some((column, physical_field))) } } @@ -654,10 +630,42 @@ mod tests { Array, BooleanArray, GenericListArray, Int32Array, Int64Array, RecordBatch, RecordBatchOptions, StringArray, StringViewArray, StructArray, }; - use arrow::datatypes::{Fields, Schema}; + use arrow::datatypes::{Field, Fields, Schema}; use datafusion_common::{assert_contains, record_batch}; use datafusion_expr::Operator; - use datafusion_physical_expr::expressions::{Column, Literal, col, lit}; + use datafusion_physical_expr::expressions::{Column, Literal, col}; + + fn assert_cast_expr(expr: &Arc) -> &CastExpr { + expr.as_any() + .downcast_ref::() + .expect("Expected CastExpr") + } + + fn assert_cast_column(cast_expr: &CastExpr, name: &str, index: usize) { + let inner_col = cast_expr + .expr() + .as_any() + .downcast_ref::() + .expect("Expected inner Column"); + assert_eq!(inner_col.name(), name); + assert_eq!(inner_col.index(), index); + } + + fn make_stale_index_cast_adapter() -> Arc { + let physical_schema = Arc::new(Schema::new(vec![ + Field::new("b", DataType::Binary, true), + Field::new("a", DataType::Int32, false), + ])); + + let logical_schema = Arc::new(Schema::new(vec![ + Field::new("a", DataType::Int64, false), + Field::new("b", DataType::Binary, true), + ])); + + DefaultPhysicalExprAdapterFactory + .create(logical_schema, physical_schema) + .unwrap() + } fn create_test_schema() -> (Schema, Schema) { let physical_schema = Schema::new(vec![ @@ -704,23 +712,10 @@ mod tests { .unwrap(); let result = adapter.rewrite(Arc::new(Column::new("a", 0)))?; - let cast = result - .as_any() - .downcast_ref::() - .expect("Expected CastExpr"); - - assert_eq!(cast.target_field().data_type(), &DataType::Int64); - assert!(!cast.target_field().is_nullable()); - assert_eq!( - cast.target_field() - .metadata() - .get("logical_meta") - .map(String::as_str), - Some("1") - ); // Ensure the expression preserves the logical field nullability/metadata. let return_field = result.return_field(physical_schema.as_ref())?; + assert_eq!(return_field.data_type(), &DataType::Int64); assert!(!return_field.is_nullable()); assert_eq!( return_field.metadata().get("logical_meta").map(String::as_str), @@ -757,32 +752,35 @@ mod tests { ); let result = adapter.rewrite(Arc::new(expr)).unwrap(); - println!("Rewritten expression: {result}"); + let outer = result + .as_any() + .downcast_ref::() + .expect("Expected outer BinaryExpr"); + assert_eq!(*outer.op(), Operator::Or); - let expected = expressions::BinaryExpr::new( - Arc::new(CastExpr::new_with_target_field( - Arc::new(Column::new("a", 0)), - Arc::new(Field::new("a", DataType::Int64, false)), - None, - )), - Operator::Plus, - Arc::new(Literal::new(ScalarValue::Int64(Some(5)))), - ); - let expected = Arc::new(expressions::BinaryExpr::new( - Arc::new(expected), - Operator::Or, - Arc::new(expressions::BinaryExpr::new( - lit(ScalarValue::Float64(None)), // c is missing, so it becomes null - Operator::Gt, - Arc::new(Literal::new(ScalarValue::Float64(Some(0.0)))), - )), - )) as Arc; + let left = outer + .left() + .as_any() + .downcast_ref::() + .expect("Expected left BinaryExpr"); + assert_eq!(*left.op(), Operator::Plus); - assert_eq!( - result.to_string(), - expected.to_string(), - "The rewritten expression did not match the expected output" - ); + let left_cast = assert_cast_expr(left.left()); + assert_eq!(left_cast.target_field().data_type(), &DataType::Int64); + assert_cast_column(left_cast, "a", 0); + + let right = outer + .right() + .as_any() + .downcast_ref::() + .expect("Expected right BinaryExpr"); + assert_eq!(*right.op(), Operator::Gt); + let null_literal = right + .left() + .as_any() + .downcast_ref::() + .expect("Expected null literal"); + assert_eq!(*null_literal.value(), ScalarValue::Float64(None)); } #[test] @@ -1657,8 +1655,7 @@ mod tests { Field::new("b", DataType::Utf8, true), ]); - let factory = DefaultPhysicalExprAdapterFactory; - let adapter = factory + let adapter = DefaultPhysicalExprAdapterFactory .create(Arc::new(logical_schema), Arc::new(physical_schema)) .unwrap(); @@ -1668,19 +1665,10 @@ mod tests { let result = adapter.rewrite(column_expr).unwrap(); // Should be a CastExpr - let cast_expr = result - .as_any() - .downcast_ref::() - .expect("Expected CastExpr"); + let cast_expr = assert_cast_expr(&result); // Verify the inner column points to the correct physical index (1) - let inner_col = cast_expr - .expr() - .as_any() - .downcast_ref::() - .expect("Expected inner Column"); - assert_eq!(inner_col.name(), "a"); - assert_eq!(inner_col.index(), 1); // Physical index is 1 + assert_cast_column(cast_expr, "a", 1); // Verify cast types assert_eq!( @@ -1691,37 +1679,13 @@ mod tests { #[test] fn test_rewrite_resolves_physical_column_by_name_before_casting() { - // Physical schema has column `a` at index 1; index 0 is an incompatible type. - let physical_schema = Arc::new(Schema::new(vec![ - Field::new("b", DataType::Binary, true), - Field::new("a", DataType::Int32, false), - ])); - - let logical_schema = Arc::new(Schema::new(vec![ - Field::new("a", DataType::Int64, false), - Field::new("b", DataType::Binary, true), - ])); - - let factory = DefaultPhysicalExprAdapterFactory; - let adapter = factory - .create(Arc::clone(&logical_schema), Arc::clone(&physical_schema)) - .unwrap(); + let adapter = make_stale_index_cast_adapter(); // Deliberately provide the wrong index for column `a`. // Regression: this must still resolve against physical field `a` by name. let rewritten = adapter.rewrite(Arc::new(Column::new("a", 0))).unwrap(); - let cast_expr = rewritten - .as_any() - .downcast_ref::() - .expect("Expected CastExpr"); - - let inner_col = cast_expr - .expr() - .as_any() - .downcast_ref::() - .expect("Expected inner Column"); - assert_eq!(inner_col.name(), "a"); - assert_eq!(inner_col.index(), 1); + let cast_expr = assert_cast_expr(&rewritten); + assert_cast_column(cast_expr, "a", 1); assert_eq!(cast_expr.target_field().data_type(), &DataType::Int64); } } From d9d255b2f417d4da29d1af2220ac9da6760c9519 Mon Sep 17 00:00:00 2001 From: Siew Kam Onn Date: Thu, 9 Apr 2026 10:28:58 +0800 Subject: [PATCH 4/5] Refactor rewrite_column fast-path control flow Simplify the control flow in the rewrite_column fast path with straightforward if checks around fields_match. Replace make_stale_index_cast_adapter() with a reusable stale_index_cast_schemas() fixture, allowing tests to choose between schemas or an adapter as needed. --- .../src/schema_rewriter.rs | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/datafusion/physical-expr-adapter/src/schema_rewriter.rs b/datafusion/physical-expr-adapter/src/schema_rewriter.rs index d62a9857469c7..c6604ceeefb59 100644 --- a/datafusion/physical-expr-adapter/src/schema_rewriter.rs +++ b/datafusion/physical-expr-adapter/src/schema_rewriter.rs @@ -423,13 +423,13 @@ impl DefaultPhysicalExprAdapterRewriter { }; let fields_match = logical_field == physical_field.as_ref(); - match (resolved_column.index() == column.index(), fields_match) { - (true, true) => return Ok(Transformed::no(expr)), - (_, true) => { - // If the fields match (including metadata/nullability), we can use the column as is - return Ok(Transformed::yes(Arc::new(resolved_column))); + if fields_match { + if resolved_column.index() == column.index() { + return Ok(Transformed::no(expr)); } - (_, false) => {} + + // If the fields match (including metadata/nullability), we can use the column as is + return Ok(Transformed::yes(Arc::new(resolved_column))); } // We need a cast expression whenever the logical and physical fields differ, @@ -651,7 +651,7 @@ mod tests { assert_eq!(inner_col.index(), index); } - fn make_stale_index_cast_adapter() -> Arc { + fn stale_index_cast_schemas() -> (SchemaRef, SchemaRef) { let physical_schema = Arc::new(Schema::new(vec![ Field::new("b", DataType::Binary, true), Field::new("a", DataType::Int32, false), @@ -662,9 +662,7 @@ mod tests { Field::new("b", DataType::Binary, true), ])); - DefaultPhysicalExprAdapterFactory - .create(logical_schema, physical_schema) - .unwrap() + (logical_schema, physical_schema) } fn create_test_schema() -> (Schema, Schema) { @@ -1679,7 +1677,10 @@ mod tests { #[test] fn test_rewrite_resolves_physical_column_by_name_before_casting() { - let adapter = make_stale_index_cast_adapter(); + let (logical_schema, physical_schema) = stale_index_cast_schemas(); + let adapter = DefaultPhysicalExprAdapterFactory + .create(logical_schema, physical_schema) + .unwrap(); // Deliberately provide the wrong index for column `a`. // Regression: this must still resolve against physical field `a` by name. From 76a1234b74b24c8704236978a18b5b876cb894e5 Mon Sep 17 00:00:00 2001 From: Siew Kam Onn Date: Thu, 9 Apr 2026 10:33:47 +0800 Subject: [PATCH 5/5] feat: improve formatting and readability in schema_rewriter.rs - Refactored the allocation of `physical_field` for better clarity. - Enhanced the readability of the metadata assertions in tests for improved code understanding. --- .../physical-expr-adapter/src/schema_rewriter.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/datafusion/physical-expr-adapter/src/schema_rewriter.rs b/datafusion/physical-expr-adapter/src/schema_rewriter.rs index c6604ceeefb59..f055c8298bc11 100644 --- a/datafusion/physical-expr-adapter/src/schema_rewriter.rs +++ b/datafusion/physical-expr-adapter/src/schema_rewriter.rs @@ -481,8 +481,11 @@ impl DefaultPhysicalExprAdapterRewriter { Column::new_with_schema(column.name(), self.physical_file_schema.as_ref())? }; - let physical_field = - Arc::new(self.physical_file_schema.field(physical_column_index).clone()); + let physical_field = Arc::new( + self.physical_file_schema + .field(physical_column_index) + .clone(), + ); Ok(Some((column, physical_field))) } @@ -716,7 +719,10 @@ mod tests { assert_eq!(return_field.data_type(), &DataType::Int64); assert!(!return_field.is_nullable()); assert_eq!( - return_field.metadata().get("logical_meta").map(String::as_str), + return_field + .metadata() + .get("logical_meta") + .map(String::as_str), Some("1") );