Jefffrey commented on code in PR #20012:
URL: https://github.com/apache/datafusion/pull/20012#discussion_r2766696916
##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -598,13 +598,32 @@ impl ExprSchemable for Expr {
)
})?;
- let arguments = args
+ let coerced_values: Vec<Option<ScalarValue>> = args
.iter()
- .map(|e| match e {
- Expr::Literal(sv, _) => Some(sv),
- _ => None,
+ .zip(new_fields.iter())
+ .map(|(expr, field)| {
+ let mut current_expr = expr;
+
+ // Loop to remove all casting layers
+ loop {
+ match current_expr {
+ Expr::Cast(cast) => current_expr = &cast.expr,
+ Expr::TryCast(cast) => current_expr =
&cast.expr,
+ _ => break,
+ }
+ }
+
+ let literal = if let Expr::Literal(sv, _) =
current_expr {
+ Some(sv)
+ } else {
+ None
+ };
+
+ literal.map(|sv|
sv.cast_to(field.data_type())).transpose()
Review Comment:
Yes I was still thinking this over; even if we change this to unwrap one
layer of casting, i.e. account for type coercion adding one layer of casting,
it would change behaviour for some functions like `arrow_cast` which expect
only a literal, even if the literal is wrapped in a cast that technically can
be evaluated during planning 🤔
```sql
> select arrow_cast(1, cast('Utf8' as varchar));
Execution error: arrow_cast requires its second argument to be a non-empty
constant string
```
- It's a minor detail but we probably wouldn't want this to start passing
So it would be nice to try delve deeper for a root cause fix, if you're up
to it.
I've linked #13825 above which relates, but @alamb I think this may also tie
in with #12604? I see you left a comment there. But there is also #18845, which
although is more of a performance lens, it would likely help here in terms of
correctness too 🤔
--
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]
