goldmedal commented on code in PR #13372:
URL: https://github.com/apache/datafusion/pull/13372#discussion_r1880062974


##########
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:
   Maybe we can refer to the behavior of `Postgres` or `DuckDB`. I listed the 
signature of the timestamp function `age` from their system catalogs for 
reference.
   
   ### Postgres
   ```sql
   test=# SELECT proname, proargtypes FROM pg_proc WHERE proname = 'age';
    proname | proargtypes 
   ---------+-------------
    age     | 28
    age     | 1114
    age     | 1184
    age     | 1114 1114
    age     | 1184 1184
   (5 rows)
   
   test=# SELECT oid, typname FROM pg_type WHERE oid IN (28, 1184, 1114);
    oid  |   typname   
   ------+-------------
      28 | xid
    1114 | timestamp
    1184 | timestamptz
   (3 rows)
   ```
   
   ### DuckDB
   ```sql
   D SELECT function_name, parameter_types 
     FROM duckdb_functions() 
    WHERE function_name = 'age';
   ┌───────────────┬──────────────────────────────────────────────────────┐
   │ function_name │                   parameter_types                    │
   │    varchar    │                      varchar[]                       │
   ├───────────────┼──────────────────────────────────────────────────────┤
   │ age           │ [TIMESTAMP]                                          │
   │ age           │ [TIMESTAMP, TIMESTAMP]                               │
   │ age           │ [TIMESTAMP WITH TIME ZONE, TIMESTAMP WITH TIME ZONE] │
   │ age           │ [TIMESTAMP WITH TIME ZONE]                           │
   └───────────────┴──────────────────────────────────────────────────────┘
   ```
   
   If you try to input an unsupported type value, you may encounter an error 
like the following:  
   
   ```sql
   D SELECT age('2001-01-01 18:00:00');
   Binder Error: Could not choose a best candidate function for the function 
call "age(STRING_LITERAL)". In order to select one, please add explicit type 
casts.
       Candidate functions:
       age(TIMESTAMP WITH TIME ZONE) -> INTERVAL
       age(TIMESTAMP) -> INTERVAL
   
   LINE 1: SELECT age('2001-01-01 18:00:00');
   ```
   
   Both systems treat `TIMESTAMP` and `TIMESTAMP WITH TIME ZONE` as distinct 
types in the high level.  
   
   The advantage of separating these types is that it allows for stricter 
type-checking when matching a function's signature. This reduces the likelihood 
of developers failing to correctly handle type checks when implementing 
timestamp functions.
   
   Honestly, I'm not sure how complex it would become if we separated them 🤔 . 
If it requires significant effort, I'm fine with keeping the current design.



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

Reply via email to