alamb commented on code in PR #13356:
URL: https://github.com/apache/datafusion/pull/13356#discussion_r1852957074
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -515,6 +521,23 @@ fn get_valid_types(
vec![vec![valid_type; *number]]
}
+ TypeSignature::Comparable(num) => {
+ function_length_check(current_types.len(), *num)?;
+ let mut target_type = current_types[0].to_owned();
+ for data_type in current_types.iter().skip(1) {
+ if let Some(dt) = comparison_coercion_numeric(&target_type,
data_type) {
Review Comment:
It seems like `TypeSignature::Comparable` has different rules that, for
example, `=` or `<` (it prefers numeric)
I think this is fine, but should probably be documented better / more
explicitly as it might be expected
Also the coercion / defaulting to `Utf8` might also be a good thing to
document
##########
datafusion/sqllogictest/test_files/nullif.slt:
##########
@@ -97,11 +97,35 @@ SELECT NULLIF(1, 3);
----
1
-query I
+query T
SELECT NULLIF(NULL, NULL);
----
NULL
+query R
+select nullif(1, 1.2);
+----
+1
+
+query R
+select nullif(1.0, 2);
+----
+1
+
+query error DataFusion error: Error during planning: Internal error: Failed to
match any signature, errors: Error during planning: The signature expected
NativeType::String but received NativeType::Int64
+select nullif(2, 'a');
+
+
+query T
+select nullif('2', '3');
+----
+2
+
+# TODO: support numeric string
+# This query success in Postgres and DuckDB
+query error DataFusion error: Error during planning: Internal error: Failed to
match any signature, errors: Error during planning: The signature expected
NativeType::String but received NativeType::Int64
+select nullif(2, '1');
Review Comment:
I just double checked on this branch and it seems good to me (supports
things reasonably)
```sql
Running `target/debug/datafusion-cli`
DataFusion CLI v43.0.0
> select nullif(2, '1');
+----------------------------+
| nullif(Int64(2),Utf8("1")) |
+----------------------------+
| 2 |
+----------------------------+
1 row(s) fetched.
Elapsed 0.027 seconds.
> select nullif('1'::varchar, 2);
+----------------------------+
| nullif(Utf8("1"),Int64(2)) |
+----------------------------+
| 1 |
+----------------------------+
1 row(s) fetched.
Elapsed 0.007 seconds.
```
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -113,6 +113,8 @@ pub enum TypeSignature {
/// arguments like `vec![DataType::Int32]` or `vec![DataType::Float32]`
/// since i32 and f32 can be casted to f64
Coercible(Vec<LogicalTypeRef>),
+ /// The number of arguments that are comparable
Review Comment:
Would it be possible to define what `comparable` means in this context?
--
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]
