fix: coerce scalar for between #3327
Conversation
There was a problem hiding this comment.
Sweet! This probably also closes #3311
Feel free to ignore my review comment if it is complicated / doesn't work. I think the vast majority of BETWEEN clauses are going to be BETWEEN <literal> AND <literal> anyways.
| let low = if matches!(low.as_ref(), Expr::Literal(_)) { | ||
| Box::new(resolve_value(low.as_ref(), &inner_expr_type)?) | ||
| } else { | ||
| low.clone() | ||
| }; | ||
| let high = if matches!(high.as_ref(), Expr::Literal(_)) { | ||
| Box::new(resolve_value(high.as_ref(), &inner_expr_type)?) | ||
| } else { | ||
| high.clone() | ||
| }; |
There was a problem hiding this comment.
Is it possible to use something more generic like let low = coerce_expr(left.as_ref(), &inner_expr_type)?;
(note: I don't know off the top of my head if this works or not)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3327 +/- ##
==========================================
- Coverage 79.01% 78.90% -0.11%
==========================================
Files 246 246
Lines 87628 87845 +217
Branches 87628 87845 +217
==========================================
+ Hits 69238 69318 +80
- Misses 15523 15650 +127
- Partials 2867 2877 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
for example, current
str > 10will throw exception. butstr BETWEEN 10 AND 20will not throw exception.with this PR check type coerce for between clause.