martin-g commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2908005743
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -158,7 +158,7 @@ pub enum Arity {
pub enum TypeSignature {
/// One or more arguments of a common type out of a list of valid types.
///
- /// For functions that take no arguments (e.g. `random()` see
[`TypeSignature::Nullary`]).
+ /// For functions that take no arguments (e.g. `random()`), see
[`TypeSignature::Nullary`]).
Review Comment:
```suggestion
/// For functions that take no arguments (e.g. `random()`), see
[`TypeSignature::Nullary`].
```
##########
datafusion/expr/src/type_coercion/other.rs:
##########
@@ -17,38 +17,58 @@
use arrow::datatypes::DataType;
-use super::binary::comparison_coercion;
+use super::binary::{comparison_coercion, type_union_coercion};
+
+/// Fold `coerce_fn` over `types`, starting from `initial_type`.
+fn fold_coerce(
+ initial_type: &DataType,
+ types: &[DataType],
+ coerce_fn: fn(&DataType, &DataType) -> Option<DataType>,
+) -> Option<DataType> {
+ types
+ .iter()
+ .try_fold(initial_type.clone(), |left_type, right_type| {
+ coerce_fn(&left_type, right_type)
+ })
+}
/// Attempts to coerce the types of `list_types` to be comparable with the
-/// `expr_type`.
-/// Returns the common data type for `expr_type` and `list_types`
+/// `expr_type` for IN list predicates.
+/// Returns the common data type for `expr_type` and `list_types`.
+///
+/// Uses comparison coercion because `x IN (a, b)` is semantically equivalent
+/// to `x = a OR x = b`.
pub fn get_coerce_type_for_list(
expr_type: &DataType,
list_types: &[DataType],
) -> Option<DataType> {
- list_types
- .iter()
- .try_fold(expr_type.clone(), |left_type, right_type| {
- comparison_coercion(&left_type, right_type)
- })
+ fold_coerce(expr_type, list_types, comparison_coercion)
+}
+
+/// Find a common coerceable type for `CASE expr WHEN val1 WHEN val2 ...`
+/// conditions. Returns the common type for `case_type` and all `when_types`.
+///
+/// Uses comparison coercion because `CASE expr WHEN val` is semantically
+/// equivalent to `expr = val`.
+pub fn get_coerce_type_for_case_when(
+ when_types: &[DataType],
+ case_type: &DataType,
+) -> Option<DataType> {
+ fold_coerce(case_type, when_types, comparison_coercion)
}
-/// Find a common coerceable type for all `when_or_then_types` as well
-/// and the `case_or_else_type`, if specified.
-/// Returns the common data type for `when_or_then_types` and
`case_or_else_type`
+/// Find a common coerceable type for CASE THEN/ELSE result expressions.
+/// Returns the common data type for `then_types` and `else_type`.
+///
+/// Uses type union coercion because the result branches must be brought to a
+/// common type (like UNION), not compared.
pub fn get_coerce_type_for_case_expression(
- when_or_then_types: &[DataType],
- case_or_else_type: Option<&DataType>,
+ then_types: &[DataType],
+ else_type: Option<&DataType>,
) -> Option<DataType> {
- let case_or_else_type = match case_or_else_type {
- None => when_or_then_types[0].clone(),
- Some(data_type) => data_type.clone(),
+ let (initial_type, remaining) = match else_type {
+ None => (&then_types[0], &then_types[1..]),
Review Comment:
```suggestion
None => then_types.split_first()?,
```
this would be more safe since it returns None if the slice has no items
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -184,38 +184,39 @@ pub enum TypeSignature {
Uniform(usize, Vec<DataType>),
/// One or more arguments with exactly the specified types in order.
///
- /// For functions that take no arguments (e.g. `random()`) use
[`TypeSignature::Nullary`].
+ /// For functions that take no arguments (e.g. `random()`), use
[`TypeSignature::Nullary`].
Exact(Vec<DataType>),
/// One or more arguments belonging to the [`TypeSignatureClass`], in
order.
///
/// [`Coercion`] contains not only the desired type but also the allowed
/// casts. For example, if you expect a function has string type, but you
/// also allow it to be casted from binary type.
///
- /// For functions that take no arguments (e.g. `random()`) see
[`TypeSignature::Nullary`].
+ /// For functions that take no arguments (e.g. `random()`), see
[`TypeSignature::Nullary`].
Coercible(Vec<Coercion>),
/// One or more arguments coercible to a single, comparable type.
///
/// Each argument will be coerced to a single type using the
- /// coercion rules described in [`comparison_coercion_numeric`].
+ /// coercion rules described in [`comparison_coercion`].
///
/// # Examples
///
/// If the `nullif(1, 2)` function is called with `i32` and `i64` arguments
/// the types will both be coerced to `i64` before the function is invoked.
///
/// If the `nullif('1', 2)` function is called with `Utf8` and `i64`
arguments
- /// the types will both be coerced to `Utf8` before the function is
invoked.
+ /// the types will both be coerced to `Int64` before the function is
invoked
+ /// (numeric is preferred over string).
///
/// Note:
- /// - For functions that take no arguments (e.g. `random()` see
[`TypeSignature::Nullary`]).
+ /// - For functions that take no arguments (e.g. `random()`), see
[`TypeSignature::Nullary`]).
Review Comment:
```suggestion
/// - For functions that take no arguments (e.g. `random()`), see
[`TypeSignature::Nullary`].
```
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -246,7 +247,7 @@ pub enum TypeSignature {
/// For example, if a function is called with (utf8, large_utf8), all
/// arguments will be coerced to `LargeUtf8`
///
- /// For functions that take no arguments (e.g. `random()` use
[`TypeSignature::Nullary`]).
+ /// For functions that take no arguments (e.g. `random()`), use
[`TypeSignature::Nullary`]).
Review Comment:
```suggestion
/// For functions that take no arguments (e.g. `random()`), use
[`TypeSignature::Nullary`].
```
--
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]
