-
Notifications
You must be signed in to change notification settings - Fork 525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(batch): scan_range should cast literal type to column's type #3470
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3470 +/- ##
==========================================
- Coverage 74.42% 74.40% -0.03%
==========================================
Files 769 769
Lines 107270 107303 +33
==========================================
+ Hits 79840 79842 +2
- Misses 27430 27461 +31
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
.literal(); | ||
let lit = LiteralExpression::try_from(bound.value.as_ref().unwrap()).unwrap(); | ||
|
||
let datum = cast(lit, bound_ty.clone()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be better to store Datum
with the correct data type in ScanRange
instead of converting it here. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but we don't have eval in fe yet? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but we don't have eval in fe yet? 🤔
We need one!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved as a temporary fix.
Hey @xxchan, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by commenting with |
@Mergifyio requeue |
❌ This pull request head commit has not been previously disembarked from queue. |
I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.
What's changed and what's your intention?
Fix #3452: point get/range scan returns empty results.
The problem is the literal's type mismatches with the column's type. In
scan_range.slt
, either makeuser_id
BIGINT or addORDER BY orders_count
(which is actually BIGINT) can trigger this problem.Not sure my
cast
is idiomatic 🥸 (Maybe a direct cast onDatum
is betterChecklist
./risedev check
(or alias,./risedev c
)Refer to a related PR or issue link (optional)