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