alamb commented on code in PR #10268:
URL: https://github.com/apache/datafusion/pull/10268#discussion_r1589137130
##########
datafusion/sqllogictest/test_files/coalesce.slt:
##########
@@ -23,7 +23,7 @@ select coalesce(1, 2, 3);
1
# test with first null
-query IT
+query ?T
Review Comment:
What happened here? Does this say the type of `coalesce(null, 3, 2, 1)` is
now Null? But the result of `arrow_typeof(coalesce(null, 3, 2, 1))` is still
Int64 😕
##########
datafusion/sqllogictest/test_files/coalesce.slt:
##########
@@ -209,28 +209,20 @@ select
[3, 4] List(Field { name: "item", data_type: Int64, nullable: true, dict_id:
0, dict_is_ordered: false, metadata: {} })
# TODO: after switch signature of array to the same with coalesce, this query
should be fixed
-query ?T
+query error
select
coalesce(array[1, 2], array[arrow_cast(3, 'Int32'), arrow_cast(4, 'Int32')]),
arrow_typeof(coalesce(array[1, 2], array[arrow_cast(3, 'Int32'),
arrow_cast(4, 'Int32')]));
-----
-[1, 2] List(Field { name: "item", data_type: Int64, nullable: true, dict_id:
0, dict_is_ordered: false, metadata: {} })
statement ok
create table test1 as values (arrow_cast('foo', 'Dictionary(Int32, Utf8)')),
(null);
# Dictionary and String are not coercible
-query ?
+query error
Review Comment:
Can we please keep this functionality? (we use dictionaries heavily in IOx
-- the user doesn't really know or hopefully have to care if a column is
`Dictionary` or `String`)
I don't see why we can't keep coercing string dictionaries to strings and
visa versa
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -53,20 +54,31 @@ pub fn data_types(
}
}
- let valid_types = get_valid_types(&signature.type_signature,
current_types)?;
-
+ let mut valid_types = get_valid_types(&signature.type_signature,
current_types)?;
if valid_types
.iter()
.any(|data_type| data_type == current_types)
{
return Ok(current_types.to_vec());
}
- // Try and coerce the argument types to match the signature, returning the
- // coerced types from the first matching signature.
- for valid_types in valid_types {
- if let Some(types) = maybe_data_types(&valid_types, current_types) {
- return Ok(types);
+ // Well-supported signature that returns exact valid types.
+ if !valid_types.is_empty()
+ && matches!(signature.type_signature,
TypeSignature::VariadicEqualOrNull)
+ {
+ // exact valid types
+ assert_eq!(valid_types.len(), 1);
+ let valid_types = valid_types.swap_remove(0);
Review Comment:
Ah, got it -- thanks
##########
datafusion/sqllogictest/test_files/coalesce.slt:
##########
@@ -35,7 +35,7 @@ select coalesce(null, null);
NULL
# cast to float
-query RT
+query IT
Review Comment:
likewise this looks reasonable (coerce has changed to have type `Int64`
because the first argument is Int64. But why did the report of `arrow_typeof`
not change 🤔
--
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
