goldmedal commented on code in PR #13313:
URL: https://github.com/apache/datafusion/pull/13313#discussion_r1835329319
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -258,20 +260,65 @@ impl TypeSignature {
.iter()
.flat_map(|type_sig| type_sig.get_possible_types())
.collect(),
+ TypeSignature::Uniform(arg_count, types) => types
+ .iter()
+ .map(|data_type| vec![data_type.clone(); *arg_count])
+ .collect(),
+ TypeSignature::Coercible(types) => types
+ .iter()
+ .map(|logical_type| get_data_types(logical_type.native()))
+ .multi_cartesian_product()
+ .collect(),
+ TypeSignature::Variadic(types) => types
+ .iter()
+ .cloned()
+ .map(|data_type| vec![data_type; types.len()])
+ .collect(),
+ TypeSignature::Numeric(arg_count) => NUMERICS
+ .iter()
+ .map(|numeric_type| vec![numeric_type.clone(); *arg_count])
+ .collect(),
+ TypeSignature::String(arg_count) => STRINGS
+ .iter()
+ .map(|string_type| vec![string_type.clone(); *arg_count])
+ .collect(),
// TODO: Implement for other types
- TypeSignature::Uniform(_, _)
- | TypeSignature::Coercible(_)
- | TypeSignature::Any(_)
- | TypeSignature::Variadic(_)
+ TypeSignature::Any(_)
| TypeSignature::VariadicAny
- | TypeSignature::UserDefined
| TypeSignature::ArraySignature(_)
- | TypeSignature::Numeric(_)
- | TypeSignature::String(_) => vec![],
+ | TypeSignature::UserDefined => vec![],
}
}
}
+fn get_data_types(native_type: &NativeType) -> Vec<DataType> {
+ match native_type {
+ NativeType::Null => vec![DataType::Null],
+ NativeType::Boolean => vec![DataType::Boolean],
+ NativeType::Int8 => vec![DataType::Int8],
+ NativeType::Int16 => vec![DataType::Int16],
+ NativeType::Int32 => vec![DataType::Int32],
+ NativeType::Int64 => vec![DataType::Int64],
+ NativeType::UInt8 => vec![DataType::UInt8],
+ NativeType::UInt16 => vec![DataType::UInt16],
+ NativeType::UInt32 => vec![DataType::UInt32],
+ NativeType::UInt64 => vec![DataType::UInt64],
+ NativeType::Float16 => vec![DataType::Float16],
+ NativeType::Float32 => vec![DataType::Float32],
+ NativeType::Float64 => vec![DataType::Float64],
+ NativeType::Date => vec![DataType::Date32, DataType::Date64],
+ NativeType::Binary => vec![
+ DataType::Binary,
+ DataType::LargeBinary,
+ DataType::BinaryView,
+ ],
+ NativeType::String => {
+ vec![DataType::Utf8, DataType::LargeUtf8, DataType::Utf8View]
+ }
+ _ => unreachable!(),
Review Comment:
Here isn't unreachable. Other native types are waiting to be supported. We
can return an empty vector instead of a panic.
```suggestion
// TODO: support other native types
_ => vec![],
```
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -258,20 +260,65 @@ impl TypeSignature {
.iter()
.flat_map(|type_sig| type_sig.get_possible_types())
.collect(),
+ TypeSignature::Uniform(arg_count, types) => types
+ .iter()
+ .map(|data_type| vec![data_type.clone(); *arg_count])
+ .collect(),
+ TypeSignature::Coercible(types) => types
+ .iter()
+ .map(|logical_type| get_data_types(logical_type.native()))
+ .multi_cartesian_product()
+ .collect(),
+ TypeSignature::Variadic(types) => types
+ .iter()
+ .cloned()
+ .map(|data_type| vec![data_type; types.len()])
+ .collect(),
+ TypeSignature::Numeric(arg_count) => NUMERICS
+ .iter()
+ .map(|numeric_type| vec![numeric_type.clone(); *arg_count])
+ .collect(),
+ TypeSignature::String(arg_count) => STRINGS
Review Comment:
It seems `type_coercion::aggregates::STRINGS` misses `DataType::Utf8View`.
Could you help to add it?
```rust
pub static STRINGS: &[DataType] = &[DataType::Utf8, DataType::LargeUtf8,
DataType::Utf8View];
```
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -258,20 +260,65 @@ impl TypeSignature {
.iter()
.flat_map(|type_sig| type_sig.get_possible_types())
.collect(),
+ TypeSignature::Uniform(arg_count, types) => types
+ .iter()
+ .map(|data_type| vec![data_type.clone(); *arg_count])
+ .collect(),
+ TypeSignature::Coercible(types) => types
+ .iter()
+ .map(|logical_type| get_data_types(logical_type.native()))
+ .multi_cartesian_product()
+ .collect(),
+ TypeSignature::Variadic(types) => types
+ .iter()
+ .cloned()
+ .map(|data_type| vec![data_type; types.len()])
Review Comment:
```suggestion
.map(|data_type| vec![data_type])
```
The length doesn't indicate the number of arguments. Variadic means the
number of arguments is arbitrary, so we can provide just one element in the
vector
--
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]
