alamb commented on code in PR #14440:
URL: https://github.com/apache/datafusion/pull/14440#discussion_r1947837053
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -127,12 +132,11 @@ pub enum TypeSignature {
Exact(Vec<DataType>),
/// One or more arguments belonging to the [`TypeSignatureClass`], in
order.
///
- /// For example, `Coercible(vec![logical_float64()])` accepts
- /// arguments like `vec![Int32]` or `vec![Float32]`
- /// since i32 and f32 can be cast to f64
+ /// [`Coercion`] contains not only the desired type but also the allowed
casts.
Review Comment:
this API makes a lot of sense to me-- in fact I think it is pretty close to
being able to express most other signatures.
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -225,6 +229,45 @@ impl Display for TypeSignatureClass {
}
}
+impl TypeSignatureClass {
+ /// Returns the default cast type for the given `TypeSignatureClass`.
Review Comment:
I am not familiar what a "default cast type" is
I looked at the definition of LogicalType:defaut_cast_for and that didn't
help me either (no comments)
https://github.com/apache/datafusion/blob/08d3b656d761a8e0d9e7f7c35c7b76cfec00e095/datafusion/common/src/types/native.rs#L201-L200
🤔
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -431,6 +463,35 @@ impl TypeSignature {
}
}
+fn get_possible_types_from_signature_classes(
Review Comment:
this would make more sense to me as a method on `TypeSignatureClass`
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -460,6 +521,44 @@ fn get_data_types(native_type: &NativeType) ->
Vec<DataType> {
}
}
+#[derive(Debug, Clone, Eq, PartialOrd)]
+pub struct Coercion {
+ pub desired_type: TypeSignatureClass,
+ pub allowed_casts: Vec<TypeSignatureClass>,
Review Comment:
What is `allowed_casts`s? Does that represent the source types that this
coercion applies to?
If so, perhaps we could name this filed `allowed_source_types`
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -460,6 +521,44 @@ fn get_data_types(native_type: &NativeType) ->
Vec<DataType> {
}
}
+#[derive(Debug, Clone, Eq, PartialOrd)]
+pub struct Coercion {
Review Comment:
I really like the idea of Coercion.
Can this idea be used for user defined coercions or coercions to specific
(Arrow) types?
Also, is there any way to allow users to provide their own coercion rules?
For example, if Sail / @shehabgamin wants to support converting numeric values
to strings automatically, would he be express that?
--
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
