alamb commented on code in PR #12421:
URL: https://github.com/apache/datafusion/pull/12421#discussion_r1754831765


##########
datafusion/expr-common/src/signature.rs:
##########
@@ -127,7 +129,62 @@ pub enum TypeSignature {
     Numeric(usize),
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+// manual implementation of `PartialOrd`

Review Comment:
   I think we could automatically derive `PartialOrd` for these enums rather 
than implement them manually. 
   
   using `derive` I think would be easier to maintain over the long term (aka 
it is easier to add new variants to the enum without having ti update the 
partial ord implemetnation)
   
   I tried this locally and it seems to work:
   
   ```diff
   andrewlamb@Andrews-MacBook-Pro-2:~/Software/datafusion$ git diff
   diff --git a/datafusion/expr-common/src/signature.rs 
b/datafusion/expr-common/src/signature.rs
   index 2043757a4..3f0f61f2f 100644
   --- a/datafusion/expr-common/src/signature.rs
   +++ b/datafusion/expr-common/src/signature.rs
   @@ -84,7 +84,7 @@ pub enum Volatility {
    ///   DataType::Timestamp(TimeUnit::Nanosecond, 
Some(TIMEZONE_WILDCARD.into())),
    /// ]);
    /// ```
   -#[derive(Debug, Clone, PartialEq, Eq, Hash)]
   +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd)]
    pub enum TypeSignature {
        /// One or more arguments of an common type out of a list of valid 
types.
        ///
   @@ -127,7 +127,7 @@ pub enum TypeSignature {
        Numeric(usize),
    }
   
   -#[derive(Debug, Clone, PartialEq, Eq, Hash)]
   +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd)]
    pub enum ArrayFunctionSignature {
        /// Specialized Signature for ArrayAppend and similar functions
        /// The first argument should be List/LargeList/FixedSizedList, and the 
second argument should be non-list or list.
   ```



-- 
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]

Reply via email to