findepi commented on code in PR #13240:
URL: https://github.com/apache/datafusion/pull/13240#discussion_r1827423960
##########
datafusion/common/src/types/native.rs:
##########
@@ -392,8 +398,38 @@ impl From<DataType> for NativeType {
}
}
-impl From<&DataType> for NativeType {
- fn from(value: &DataType) -> Self {
- value.clone().into()
+impl NativeType {
+ #[inline]
+ pub fn is_numeric(&self) -> bool {
+ use NativeType::*;
+ matches!(
+ self,
+ UInt8
+ | UInt16
+ | UInt32
+ | UInt64
+ | Int8
+ | Int16
+ | Int32
+ | Int64
+ | Float16
+ | Float32
+ | Float64
+ | Decimal(_, _)
+ )
+ }
+
+ /// This function is the NativeType version of `can_cast_types`.
+ /// It handles general coercion rules that are widely applicable.
+ /// Avoid adding specific coercion cases here.
+ /// Aim to keep this logic as SIMPLE as possible!
+ pub fn can_cast_to(&self, target_type: &Self) -> bool {
+ // In Postgres, most functions coerce numeric strings to numeric
inputs,
+ // but they do not accept numeric inputs as strings.
+ if self.is_numeric() && target_type == &NativeType::String {
+ return false;
+ }
+
+ true
Review Comment:
Default for "can cast" and "can coerce" should be false.
Maps cannot coerce to numbers or lists.
Let'e have
```suggestion
false
```
here and let's define what can be converted in rules above. This will make
the code simpler to reason about
##########
datafusion/common/src/types/native.rs:
##########
@@ -392,8 +398,38 @@ impl From<DataType> for NativeType {
}
}
-impl From<&DataType> for NativeType {
- fn from(value: &DataType) -> Self {
- value.clone().into()
+impl NativeType {
+ #[inline]
+ pub fn is_numeric(&self) -> bool {
+ use NativeType::*;
+ matches!(
+ self,
+ UInt8
+ | UInt16
+ | UInt32
+ | UInt64
+ | Int8
+ | Int16
+ | Int32
+ | Int64
+ | Float16
+ | Float32
+ | Float64
+ | Decimal(_, _)
+ )
+ }
+
+ /// This function is the NativeType version of `can_cast_types`.
+ /// It handles general coercion rules that are widely applicable.
+ /// Avoid adding specific coercion cases here.
+ /// Aim to keep this logic as SIMPLE as possible!
+ pub fn can_cast_to(&self, target_type: &Self) -> bool {
+ // In Postgres, most functions coerce numeric strings to numeric
inputs,
+ // but they do not accept numeric inputs as strings.
+ if self.is_numeric() && target_type == &NativeType::String {
Review Comment:
This talks about "can cast" and "can coerce" without making clear
distinction between them.
Can we make it less ambigous and clarify whether we may "can cast" (ie does
`CAST(type_from AS type_to)` exist?) or "can coerce" (does cast exist and is
applied implicitly in various contexts?)
--
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
