Adam Curtis

Results 71 comments of Adam Curtis

https://github.com/apache/datafusion/pull/10323 is ready for review and avoids the previously discussed issues with https://github.com/apache/datafusion/pull/10221

There is a test failure in sqllogictests ``` External error: query failed: DataFusion error: Error during planning: No function matches the given name and argument types 'coalesce(Int64, Dictionary(Int32, Int8))'. You...

The test failure is from `comparison_coercion` being used in function argument type coercion for the `coalesce` function and other `Signature::VariadicEqual` functions. https://github.com/apache/datafusion/blob/7fe8eeb698802776744cbba6d71abd30cc850968/datafusion/expr/src/type_coercion/functions.rs#L186-L201

There are other places `comparison_coercion` is used so we may want to look for additional unintended side effects of this change. array prepend/append: https://github.com/apache/datafusion/blob/7fe8eeb698802776744cbba6d71abd30cc850968/datafusion/expr/src/type_coercion/functions.rs#L138 list type coercions: https://github.com/apache/datafusion/blob/7fe8eeb698802776744cbba6d71abd30cc850968/datafusion/expr/src/type_coercion/other.rs#L25-L34 `CASE WHEN...

It looks like the failing test case is no longer throwing an error after I added @jayzhan211 's suggestion: ``` DataFusion CLI v37.1.0 > select coalesce(34, arrow_cast(123, 'Dictionary(Int32, Int8)')); +----------------------------------------------------------------------------+...

I may try to approach a solution from a different angle here, to avoid needing to refactor the type coercion logic too much without any clear understanding of what the...

Or even more generally, you could ignore whether the operands are column references or constants and just have: `ValueType( :: Dictionary) ValueType( :: )` becomes: `( :: Dictionary) CAST( as...

> > I may try to approach a solution from a different angle here, to avoid needing to refactor the type coercion logic too much without any clear understanding of...

this comment also seems to hint at a possible refactor: https://github.com/apache/datafusion/blob/f47e74d6d52000eed6f9472bc2cc43a16b8814a8/datafusion/expr/src/type_coercion/functions.rs#L190-L194 we could potentially rewrite `coalesce` coercion to use `maybe_data_type` which allows restricting coercion to a list of output types...

This diff shows a list of types that coalesce can accept. There might be others but this would serve as a good starting point: https://github.com/apache/datafusion/pull/9459/files#diff-9b644c479dfb999609b40e8da6d3b2a40c7adadf88296eeefd2f803201e7ab6dL25-L43