neilconway commented on code in PR #26498:
URL: https://github.com/apache/datafusion/pull/26498#discussion_r4315954998
##########
datafusion/functions/src/core/nvl2.rs:
##########
@@ -125,20 +125,11 @@ impl ScalarUDFImpl for NVL2Func {
fn coerce_types(&self, arg_types: &[DataType]) -> Result<Vec<DataType>> {
let [tested, if_non_null, if_null] = take_function_args(self.name(),
arg_types)?;
- let new_type =
- [if_non_null, if_null]
- .iter()
- .try_fold(tested.clone(), |acc, x| {
- // `type_union_coercion` finds a loose common type; the
actual
- // coercion is done by `maybe_data_types`.
- let coerced_type = type_union_coercion(&acc, x);
- if let Some(coerced_type) = coerced_type {
- Ok(coerced_type)
- } else {
- internal_err!("Coercion from {acc} to {x} failed.")
- }
- })?;
- Ok(vec![new_type; arg_types.len()])
+ let Some(new_type) = type_union_coercion(if_non_null, if_null) else {
Review Comment:
`type_union_coercion` has the property that it depends on argument order (it
takes the TZ of the leftmost argument if the timezones of the inputs disagree):
```
CREATE TABLE t AS
SELECT i, a, arrow_cast(a, 'Timestamp(Nanosecond, Some("+05:00"))') AS b
FROM (SELECT 1 AS i,
arrow_cast(TIMESTAMP '2024-01-01 22:00:00',
'Timestamp(Nanosecond, Some("+00:00"))') AS a);
-- a and b are the same instant, typed with different zones
SELECT a, b, a = b AS same_instant FROM t;
-- +----------------------+---------------------------+--------------+
-- | a | b | same_instant |
-- +----------------------+---------------------------+--------------+
-- | 2024-01-01T22:00:00Z | 2024-01-02T03:00:00+05:00 | true |
-- +----------------------+---------------------------+--------------+
SELECT nvl2(i, a, b) AS v,
arrow_typeof(nvl2(i, a, b)) AS v_type,
CAST(nvl2(i, a, b) AS DATE) AS v_date
FROM t;
-- +----------------------+-------------------------+------------+
-- | v | v_type | v_date |
-- +----------------------+-------------------------+------------+
-- | 2024-01-01T22:00:00Z | Timestamp(ns, "+00:00") | 2024-01-01 |
-- +----------------------+-------------------------+------------+
SELECT CASE WHEN i IS NOT NULL THEN a ELSE b END AS v,
arrow_typeof(CASE WHEN i IS NOT NULL THEN a ELSE b END) AS v_type,
CAST(CASE WHEN i IS NOT NULL THEN a ELSE b END AS DATE) AS v_date
FROM t;
-- +---------------------------+-------------------------+------------+
-- | v | v_type | v_date |
-- +---------------------------+-------------------------+------------+
-- | 2024-01-02T03:00:00+05:00 | Timestamp(ns, "+05:00") | 2024-01-02 |
-- +---------------------------+-------------------------+------------+
```
To match `CASE`'s behavior, can we call `type_union_coercion(if_null,
if_non_null)` instead? And add a test case for this.
(`type_union_coercion`'s behavior here is probably not optimal but that's a
separate issue.)
--
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]
