Support DataType::Decimal for IN and NOT IN expressions#2764
Support DataType::Decimal for IN and NOT IN expressions#2764alamb merged 1 commit intoapache:masterfrom
DataType::Decimal for IN and NOT IN expressions#2764Conversation
github-actions
bot
added
logical-expr
| .collect(); | ||
| // TODO in the arrow-rs, should support NULL type to Decimal Data type | ||
| // TODO support in the arrow-rs, NULL value cast to Decimal Value | ||
| let result_type = get_coerce_type(expr_type, &list_types); |
There was a problem hiding this comment.
The IN operator is a shorthand for multiple OR conditions.
We should get the coerced data type for IN expr.
There was a problem hiding this comment.
In this issue: #2759
The type of list is BOOLEAN, INT32, and these two type are not compatible.
liukun4515
force-pushed
the
support_decimal_inlist_filter
branch
from
e641539 to
9851725
Compare
| lit(ScalarValue::Int64(Some(0))), | ||
| lit(ScalarValue::Int64(Some(1))), | ||
| lit(ScalarValue::Utf8(None)), | ||
| lit(ScalarValue::Null), |
There was a problem hiding this comment.
We can't get the coerced data type between INT64 and UTF8.
I think the type of NULL is Null
There was a problem hiding this comment.
From this test case, we can know that we should not treat Null as UTF8 data type
liukun4515
force-pushed
the
support_decimal_inlist_filter
branch
from
9851725 to
88cfa73
Compare
|
@alamb @andygrove @viirya PTAL |
| &list_expr_data_type, | ||
| &value_expr_data_type, | ||
| ) { | ||
| // TODO: Can't cast from list type to value type directly |
There was a problem hiding this comment.
we can't cast the list data type to the value data type directly, and need to coerced common data type if they are compatible.
liukun4515
force-pushed
the
support_decimal_inlist_filter
branch
from
88cfa73 to
fb3cef3
Compare
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
I left some suggestions for code improvements that I think should be done, but the logic and tests in this PR look good to me. 👍 thanks @liukun4515
liukun4515
force-pushed
the
support_decimal_inlist_filter
branch
from
f0d0d7c to
8837d9d
Compare
Codecov Report
@@ Coverage Diff @@
## master #2764 +/- ##
==========================================
+ Coverage 84.95% 85.13% +0.17%
==========================================
Files 271 272 +1
Lines 48053 48207 +154
==========================================
+ Hits 40824 41039 +215
+ Misses 7229 7168 -61
Continue to review full report at Codecov.
|
|
All comments have be resolved. |
alamb
changed the title
DataType::Decimal for IN and NOT IN expressions
4 participants
Which issue does this PR close?
Part of #2755
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?