-
Notifications
You must be signed in to change notification settings - Fork 752
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
dal2: Fix SeekableRead not read correctly #4107
Conversation
Signed-off-by: Xuanwo <github@xuanwo.io>
Thanks for the contribution! Please review the labels and make any necessary changes. |
Thanks for the contribution! Please review the labels and make any necessary changes. |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/databend/databend/3WXALENzejEVq6uBHZrRX1ynnQHL [Deployment for 9310603 canceled] |
I remember we have a test about create table select from ..., why the test passed before? |
Maybe the dataset is too small that can be fit into buffer so that there's no extra read. (under 1MB) I will add this case into stateless test too. |
It seems we need more unit tests on dal2, like |
True, I will keep improving this part. |
Codecov Report
@@ Coverage Diff @@
## main #4107 +/- ##
======================================
Coverage 57% 57%
======================================
Files 857 857
Lines 46462 46255 -207
======================================
- Hits 26707 26670 -37
+ Misses 19755 19585 -170
Continue to review full report at Codecov.
|
Signed-off-by: Xuanwo <github@xuanwo.io>
/lgtm Preferred add the unit tests in the later PR :D |
Signed-off-by: Xuanwo github@xuanwo.io
I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/
Summary
Part of #3677
The following SQL doesn't works:
Meeting EOF errors like:
This is caused by the incorrect
SeekableReader
implementation.Changelog
Test Plan
Unit Tests
Stateless Tests