kosiew commented on code in PR #16589:
URL: https://github.com/apache/datafusion/pull/16589#discussion_r2181563813
##########
datafusion/physical-expr-adapter/src/schema_rewriter.rs:
##########
@@ -234,233 +333,141 @@ mod tests {
let result = rewriter.rewrite(column_expr).unwrap();
- // Should be wrapped in a cast expression
- assert!(result.as_any().downcast_ref::<CastExpr>().is_some());
- }
-
- #[test]
- fn test_rewrite_mulit_column_expr_with_type_cast() {
- let (physical_schema, logical_schema) = create_test_schema();
- let rewriter = PhysicalExprSchemaRewriter::new(&physical_schema,
&logical_schema);
-
- // Create a complex expression: (a + 5) OR (c > 0.0) that tests the
recursive case of the rewriter
- let column_a = Arc::new(Column::new("a", 0)) as Arc<dyn PhysicalExpr>;
- let column_c = Arc::new(Column::new("c", 2)) as Arc<dyn PhysicalExpr>;
- let expr = expressions::BinaryExpr::new(
- Arc::clone(&column_a),
- Operator::Plus,
- Arc::new(expressions::Literal::new(ScalarValue::Int64(Some(5)))),
- );
- let expr = expressions::BinaryExpr::new(
- Arc::new(expr),
- Operator::Or,
- Arc::new(expressions::BinaryExpr::new(
- Arc::clone(&column_c),
- Operator::Gt,
-
Arc::new(expressions::Literal::new(ScalarValue::Float64(Some(0.0)))),
- )),
- );
-
- let result = rewriter.rewrite(Arc::new(expr)).unwrap();
- println!("Rewritten expression: {result}");
-
- let expected = expressions::BinaryExpr::new(
- Arc::new(CastExpr::new(
- Arc::new(Column::new("a", 0)),
- DataType::Int64,
- None,
- )),
- Operator::Plus,
- Arc::new(expressions::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::Null),
- Operator::Gt,
-
Arc::new(expressions::Literal::new(ScalarValue::Float64(Some(0.0)))),
- )),
+ let expected = Arc::new(CastExpr::new(
+ Arc::new(Column::new("a", 0)),
+ DataType::Int64,
+ None,
)) as Arc<dyn PhysicalExpr>;
- assert_eq!(
- result.to_string(),
- expected.to_string(),
- "The rewritten expression did not match the expected output"
- );
+ assert_eq!(result.to_string(), expected.to_string());
}
#[test]
- fn test_rewrite_missing_column() -> Result<()> {
- let (physical_schema, logical_schema) = create_test_schema();
+ fn test_rewrite_struct_column_incompatible() {
+ let physical_schema = Schema::new(vec![Field::new(
+ "data",
+ DataType::Struct(vec![Field::new("field1", DataType::Binary,
true)].into()),
+ true,
+ )]);
+
+ let logical_schema = Schema::new(vec![Field::new(
+ "data",
+ DataType::Struct(vec![Field::new("field1", DataType::Int32,
true)].into()),
+ true,
+ )]);
let rewriter = PhysicalExprSchemaRewriter::new(&physical_schema,
&logical_schema);
- let column_expr = Arc::new(Column::new("c", 2));
+ let column_expr = Arc::new(Column::new("data", 0));
- let result = rewriter.rewrite(column_expr)?;
-
- // Should be replaced with a literal null
- if let Some(literal) =
result.as_any().downcast_ref::<expressions::Literal>() {
- assert_eq!(*literal.value(), ScalarValue::Float64(None));
- } else {
- panic!("Expected literal expression");
- }
-
- Ok(())
+ let result = rewriter.rewrite(column_expr);
+ assert!(result.is_err());
+ let error_msg = result.unwrap_err().to_string();
+ assert_contains!(error_msg, "Cannot cast column 'data'");
}
#[test]
- fn test_rewrite_partition_column() -> Result<()> {
- let (physical_schema, logical_schema) = create_test_schema();
+ fn test_rewrite_struct_compatible_cast() {
+ let physical_schema = Schema::new(vec![Field::new(
+ "data",
+ DataType::Struct(
+ vec![
+ Field::new("id", DataType::Int32, false),
+ Field::new("name", DataType::Utf8, true),
+ ]
+ .into(),
+ ),
+ false,
+ )]);
+
+ let logical_schema = Schema::new(vec![Field::new(
+ "data",
+ DataType::Struct(
+ vec![
+ Field::new("id", DataType::Int64, false),
+ Field::new("name", DataType::Utf8View, true),
+ ]
+ .into(),
+ ),
+ false,
+ )]);
- let partition_fields =
- vec![Arc::new(Field::new("partition_col", DataType::Utf8, false))];
- let partition_values =
vec![ScalarValue::Utf8(Some("test_value".to_string()))];
-
- let rewriter = PhysicalExprSchemaRewriter::new(&physical_schema,
&logical_schema)
- .with_partition_columns(partition_fields, partition_values);
+ let rewriter = PhysicalExprSchemaRewriter::new(&physical_schema,
&logical_schema);
+ let column_expr = Arc::new(Column::new("data", 0));
- let column_expr = Arc::new(Column::new("partition_col", 0));
- let result = rewriter.rewrite(column_expr)?;
+ let result = rewriter.rewrite(column_expr).unwrap();
- // Should be replaced with the partition value
- if let Some(literal) =
result.as_any().downcast_ref::<expressions::Literal>() {
- assert_eq!(
- *literal.value(),
- ScalarValue::Utf8(Some("test_value".to_string()))
- );
- } else {
- panic!("Expected literal expression");
- }
+ let expected = Arc::new(CastExpr::new(
+ Arc::new(Column::new("data", 0)),
+ DataType::Struct(
+ vec![
+ Field::new("id", DataType::Int64, false),
+ Field::new("name", DataType::Utf8View, true),
+ ]
+ .into(),
+ ),
+ None,
+ )) as Arc<dyn PhysicalExpr>;
- Ok(())
+ assert_eq!(result.to_string(), expected.to_string());
}
#[test]
- fn test_rewrite_no_change_needed() -> Result<()> {
Review Comment:
Is there a reason for removing this test?
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
