alamb commented on code in PR #13372:
URL: https://github.com/apache/datafusion/pull/13372#discussion_r1876234505
##########
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:
I think some functions currently define that they take a UTC timestamp or
ANY timestamp via
https://docs.rs/datafusion/latest/datafusion/logical_expr/constant.TIMEZONE_WILDCARD.html
##########
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:
I agree it would be great to have a special type signature class display
Maybe something like `(any int)` or `Integer` or `(any timestamp)` for
Timestamp 🤔
##########
datafusion/sqllogictest/test_files/expr.slt:
##########
@@ -1100,24 +1100,22 @@ SELECT date_part('microsecond', timestamp
'2020-09-08T12:00:12.12345678+00:00')
query error DataFusion error: Internal error: unit Nanosecond not supported
SELECT date_part('nanosecond', timestamp '2020-09-08T12:00:12.12345678+00:00')
-
-query I
+# Second argument should not be string, failed in postgres too.
+query error
Review Comment:
this feels like a regression to me (even if it fails in postgres). I think
automatically coercing to a string is important for our users in InfluxDB.
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -154,6 +156,25 @@ impl TypeSignature {
}
}
+#[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Hash)]
+pub enum TypeSignatureClass {
Review Comment:
Would it be possible to add doc comments to this enum explaining what it is
and what it is used for?
##########
datafusion/common/src/types/native.rs:
##########
@@ -245,6 +245,8 @@ impl LogicalType for NativeType {
(Self::FixedSizeBinary(size), _) => FixedSizeBinary(*size),
(Self::String, LargeBinary) => LargeUtf8,
(Self::String, BinaryView) => Utf8View,
+ // We don't cast to another kind of string type if the origin one
is already a string type
Review Comment:
💯
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -290,7 +311,24 @@ impl TypeSignature {
.collect(),
TypeSignature::Coercible(types) => types
.iter()
- .map(|logical_type| get_data_types(logical_type.native()))
+ .map(|logical_type| match logical_type {
+ TypeSignatureClass::Native(l) =>
get_data_types(l.native()),
+ TypeSignatureClass::Timestamp => {
+ vec![DataType::Timestamp(TimeUnit::Nanosecond, None)]
Review Comment:
I think this should use WILDCARD to match any timestamp (not just timestamps
without timezones)
--
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]
