findepi commented on code in PR #13372:
URL: https://github.com/apache/datafusion/pull/13372#discussion_r1845172102
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -138,6 +141,48 @@ pub enum TypeSignature {
NullAry,
}
+impl TypeSignature {
+ #[inline]
+ pub fn is_one_of(&self) -> bool {
+ matches!(self, TypeSignature::OneOf(_))
+ }
+}
+
+#[derive(Debug, Clone, Eq, PartialOrd)]
+pub enum TypeSignatureClass {
+ Timestamp,
Review Comment:
We may need to treat timestamp and timestamp with zone separately 🤔
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -112,8 +114,9 @@ pub enum TypeSignature {
/// For example, `Coercible(vec![logical_float64()])` accepts
/// arguments like `vec![DataType::Int32]` or `vec![DataType::Float32]`
/// since i32 and f32 can be casted to f64
- Coercible(Vec<LogicalTypeRef>),
- /// Fixed number of arguments of arbitrary types, number should be larger
than 0
+ Coercible(Vec<TypeSignatureClass>),
Review Comment:
nice!
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -138,6 +141,48 @@ pub enum TypeSignature {
NullAry,
}
+impl TypeSignature {
+ #[inline]
+ pub fn is_one_of(&self) -> bool {
+ matches!(self, TypeSignature::OneOf(_))
+ }
+}
+
+#[derive(Debug, Clone, Eq, PartialOrd)]
+pub enum TypeSignatureClass {
+ Timestamp,
+ Date,
+ Time,
+ Interval,
+ Duration,
+ // TODO:
+ // Numeric
+ // Integer
+ Native(LogicalTypeRef),
+}
+
+// TODO: MSRV issue: Default macro doesn't work in 1.79. Use default PartialEq
macro after it is able to compile
+impl PartialEq for TypeSignatureClass {
+ fn eq(&self, other: &Self) -> bool {
+ match (self, other) {
+ (Self::Native(l0), Self::Native(r0)) => l0 == r0,
+ _ => core::mem::discriminant(self) ==
core::mem::discriminant(other),
+ }
+ }
+}
+
+impl std::hash::Hash for TypeSignatureClass {
+ fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+ core::mem::discriminant(self).hash(state);
+ }
+}
+
+impl Display for TypeSignatureClass {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ write!(f, "TypeSignatureClass::{self:?}")
Review Comment:
Display looks more verbose than Debug. Typically it's the other way around.
the produced err msg looks a bit longish, but i don't know how to make it
more readabile. thoughts?
```
Internal error: Expect
TypeSignatureClass::Native\(LogicalType\(Native\(Int64\), Int64\)\) but
received Float64
```
##########
datafusion/common/src/types/native.rs:
##########
@@ -433,4 +433,29 @@ impl NativeType {
UInt8 | UInt16 | UInt32 | UInt64 | Int8 | Int16 | Int32 | Int64
)
}
+
+ #[inline]
Review Comment:
Do we need these compiler hints?
##########
datafusion/sqllogictest/test_files/expr.slt:
##########
@@ -1096,23 +1096,27 @@ SELECT date_part('nanosecond', timestamp
'2020-09-08T12:00:12.12345678+00:00')
----
12123456780
-query R
+# Second argument should not be string, failed in postgres too.
+query error
SELECT date_part('second', '2020-09-08T12:00:12.12345678+00:00')
+
+query R
+SELECT date_part('second', timestamp '2020-09-08T12:00:12.12345678+00:00')
----
12.12345678
query R
-SELECT date_part('millisecond', '2020-09-08T12:00:12.12345678+00:00')
+SELECT date_part('millisecond', timestamp '2020-09-08T12:00:12.12345678+00:00')
Review Comment:
Why change existing test queries?
--
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
