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