Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
c79156f
remove casting for coalesce
jayzhan211 Apr 27, 2024
a36e6b2
add more test
jayzhan211 Apr 27, 2024
bf16c92
add more test
jayzhan211 Apr 27, 2024
407e3c7
crate only visibility
jayzhan211 Apr 27, 2024
03b9162
polish comment
jayzhan211 Apr 27, 2024
4abf29d
improve test
jayzhan211 Apr 27, 2024
4965e8d
backup
jayzhan211 Apr 28, 2024
81f0235
introduce new signautre for coalesce
jayzhan211 Apr 28, 2024
bae996c
cleanup
jayzhan211 Apr 28, 2024
ddf9b1c
cleanup
jayzhan211 Apr 28, 2024
c2799ea
ignore err msg
jayzhan211 Apr 28, 2024
2574896
fmt
jayzhan211 Apr 28, 2024
6a17e57
fix doc
jayzhan211 Apr 28, 2024
4cba8c5
cleanup
jayzhan211 Apr 28, 2024
f1cfb8d
add more test
jayzhan211 Apr 28, 2024
d2e83d3
switch to type_resolution coercion
jayzhan211 Apr 28, 2024
3a88ad7
Merge remote-tracking branch 'upstream/main' into fix-coelesce
jayzhan211 Apr 29, 2024
03880a3
fix i64 and u64 case
jayzhan211 Apr 29, 2024
481f548
add more tests
jayzhan211 Apr 29, 2024
dfc4176
cleanup
jayzhan211 Apr 29, 2024
46a9060
add null case
jayzhan211 Apr 29, 2024
d656645
fmt
jayzhan211 Apr 29, 2024
5683447
fix
jayzhan211 Apr 29, 2024
b949fae
rename to type_union_resolution
jayzhan211 Apr 29, 2024
5aaeb5b
add comment
jayzhan211 Apr 29, 2024
a968c0e
Merge remote-tracking branch 'upstream/main' into fix-coelesce
jayzhan211 May 1, 2024
cf679c5
Merge remote-tracking branch 'upstream/main' into fix-coelesce
jayzhan211 May 2, 2024
15471ab
cleanup
jayzhan211 May 2, 2024
e5cc46b
fix test
jayzhan211 May 2, 2024
a810e85
add comment
jayzhan211 May 2, 2024
cb16cda
rm test
jayzhan211 May 3, 2024
53bedda
Merge remote-tracking branch 'upstream/main' into fix-coelesce
jayzhan211 May 12, 2024
a37da2d
cleanup since rebase
jayzhan211 May 12, 2024
70239e0
add more test
jayzhan211 May 12, 2024
be116f8
add more test
jayzhan211 May 12, 2024
8f4e991
fix msg
jayzhan211 May 12, 2024
6a8fe6f
Merge remote-tracking branch 'upstream/main' into fix-coelesce
jayzhan211 May 14, 2024
20e618e
Merge remote-tracking branch 'upstream/main' into fix-coelesce
jayzhan211 May 14, 2024
4153593
fmt
jayzhan211 May 14, 2024
030a519
rm pure_string_coercion
jayzhan211 May 14, 2024
5b797d5
rm duplicate
jayzhan211 May 14, 2024
b954479
change type in select.slt
jayzhan211 May 25, 2024
829b5a2
fix slt
jayzhan211 May 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
ignore err msg
Signed-off-by: jayzhan211 <jayzhan211@gmail.com>
  • Loading branch information
jayzhan211 committed Apr 28, 2024
commit c2799ead9e93f36a6464708ea17adf806fd4b2b4
16 changes: 8 additions & 8 deletions datafusion/expr/src/type_coercion/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,28 +532,28 @@ pub(crate) fn comparison_binary_numeric_coercion(
}
}

pub fn decimal_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
/// Decimal coercion rules.
pub(crate) fn decimal_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
use arrow::datatypes::DataType::*;

match (lhs_type, rhs_type) {
// Prefer decimal data type over floating point for comparison operation
(Decimal128(_, _), Decimal128(_, _)) => {
get_wider_decimal_type(lhs_type, rhs_type)
}
(Decimal128(_, _), _) => get_comparison_common_decimal_type(lhs_type, rhs_type),
(_, Decimal128(_, _)) => get_comparison_common_decimal_type(rhs_type, lhs_type),
(Decimal128(_, _), _) => get_common_decimal_type(lhs_type, rhs_type),
(_, Decimal128(_, _)) => get_common_decimal_type(rhs_type, lhs_type),
(Decimal256(_, _), Decimal256(_, _)) => {
get_wider_decimal_type(lhs_type, rhs_type)
}
(Decimal256(_, _), _) => get_comparison_common_decimal_type(lhs_type, rhs_type),
(_, Decimal256(_, _)) => get_comparison_common_decimal_type(rhs_type, lhs_type),
(Decimal256(_, _), _) => get_common_decimal_type(lhs_type, rhs_type),
(_, Decimal256(_, _)) => get_common_decimal_type(rhs_type, lhs_type),
(_, _) => None,
}
}

/// Coerce `lhs_type` and `rhs_type` to a common type for the purposes of
/// a comparison operation where one is a decimal
fn get_comparison_common_decimal_type(
/// Coerce `lhs_type` and `rhs_type` to a common type.
fn get_common_decimal_type(
decimal_type: &DataType,
other_type: &DataType,
) -> Option<DataType> {
Expand Down
84 changes: 31 additions & 53 deletions datafusion/sqllogictest/test_files/functions.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1178,14 +1178,26 @@ select coalesce(null, null);
----
NULL

# cast to float
query RT
select
coalesce(1, 2.0),
arrow_typeof(coalesce(1, 2.0))
;
----
1 Float64

query RT
select
coalesce(1, arrow_cast(2.0, 'Float32')),
arrow_typeof(coalesce(1, arrow_cast(2.0, 'Float32')))
;
----
1 Float32

# test with empty args
query error
select coalesce();
----
DataFusion error: Error during planning: No function matches the given name and argument types 'coalesce()'. You might need to add explicit type casts.
Candidate functions:
coalesce(CoercibleT or NULL, .., CoercibleT or NULL)


# test with different types
query I
Expand Down Expand Up @@ -1271,14 +1283,12 @@ test
statement ok
create table test1 as values (arrow_cast('foo', 'Dictionary(Int32, Utf8)')), (null);

# test coercion string
# Dictionary and String are not coercible
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This query actually works on main so making it error seems like a regression to me

DataFusion CLI v37.1.0
> create table test1 as values (arrow_cast('foo', 'Dictionary(Int32, Utf8)')), (null);
0 row(s) fetched.
Elapsed 0.011 seconds.

> select coalesce(column1, 'none_set') from test1;
+------------------------------------------+
| coalesce(test1.column1,Utf8("none_set")) |
+------------------------------------------+
| foo                                      |
| none_set                                 |
+------------------------------------------+
2 row(s) fetched.
Elapsed 0.003 seconds.

> select coalesce(null, column1, 'none_set') from test1;
+-----------------------------------------------+
| coalesce(NULL,test1.column1,Utf8("none_set")) |
+-----------------------------------------------+
| foo                                           |
| none_set                                      |
+-----------------------------------------------+
2 row(s) fetched.
Elapsed 0.002 seconds.

query error
select coalesce(column1, 'none_set') from test1;
----
DataFusion error: Error during planning: No function matches the given name and argument types 'coalesce(Dictionary(Int32, Utf8), Utf8)'. You might need to add explicit type casts.
Candidate functions:
coalesce(CoercibleT or NULL, .., CoercibleT or NULL)

query error
select coalesce(null, column1, 'none_set') from test1;

# explicit cast to Utf8
query T
Expand All @@ -1287,14 +1297,18 @@ select coalesce(arrow_cast(column1, 'Utf8'), 'none_set') from test1;
foo
none_set

# test coercion Int
statement ok
drop table test1

# Numeric and Dictionary are not coercible
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, this also works on main:

> select coalesce(34, arrow_cast(123, 'Dictionary(Int32, Int8)'));
+----------------------------------------------------------------------------+
| coalesce(Int64(34),arrow_cast(Int64(123),Utf8("Dictionary(Int32, Int8)"))) |
+----------------------------------------------------------------------------+
| 34                                                                         |
+----------------------------------------------------------------------------+
1 row(s) fetched.
Elapsed 0.001 seconds.

query error
select coalesce(34, arrow_cast(123, 'Dictionary(Int32, Int8)'));
----
DataFusion error: Error during planning: No function matches the given name and argument types 'coalesce(Int64, Dictionary(Int32, Int8))'. You might need to add explicit type casts.
Candidate functions:
coalesce(CoercibleT or NULL, .., CoercibleT or NULL)

query error
select coalesce(arrow_cast(123, 'Dictionary(Int32, Int8)'), 34);

query error
select coalesce(null, 34, arrow_cast(123, 'Dictionary(Int32, Int8)'));

# explicit cast to Int8, and it will implicitly cast to Int64
query IT
Expand All @@ -1304,37 +1318,6 @@ select
----
123 Int64

# test with Int
query error
select coalesce(arrow_cast(123, 'Dictionary(Int32, Int8)'), 34);
----
DataFusion error: Error during planning: No function matches the given name and argument types 'coalesce(Dictionary(Int32, Int8), Int64)'. You might need to add explicit type casts.
Candidate functions:
coalesce(CoercibleT or NULL, .., CoercibleT or NULL)


# test with null
query error
select coalesce(null, 34, arrow_cast(123, 'Dictionary(Int32, Int8)'));
----
DataFusion error: Error during planning: No function matches the given name and argument types 'coalesce(Null, Int64, Dictionary(Int32, Int8))'. You might need to add explicit type casts.
Candidate functions:
coalesce(CoercibleT or NULL, .., CoercibleT or NULL)


# test with null
query error
select coalesce(null, column1, 'none_set') from test1;
----
DataFusion error: Error during planning: No function matches the given name and argument types 'coalesce(Null, Dictionary(Int32, Utf8), Utf8)'. You might need to add explicit type casts.
Candidate functions:
coalesce(CoercibleT or NULL, .., CoercibleT or NULL)


statement ok
drop table test1


statement ok
CREATE TABLE test(
c1 INT,
Expand All @@ -1356,14 +1339,9 @@ SELECT COALESCE(c1, c2) FROM test
1
NULL

# coalesce result with default value
# int and string are not coercible
query error
SELECT COALESCE(c1, c2, '-1') FROM test
----
DataFusion error: Error during planning: No function matches the given name and argument types 'coalesce(Int32, Int32, Utf8)'. You might need to add explicit type casts.
Candidate functions:
coalesce(CoercibleT or NULL, .., CoercibleT or NULL)

SELECT COALESCE(c1, c2, '-1') FROM test;

statement ok
drop table test
Expand Down