-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Minor: port some explain test to sqllogictest, add filename normalization #5252
Conversation
/// " Projection: d.b, MAX(d.a) AS max_a", | ||
/// ] | ||
/// ``` | ||
fn expand_row(mut row: Vec<String>) -> impl Iterator<Item = Vec<String>> { |
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.
This was needed otherwise sqllogictest got confused about cells that had multiple rows in them
LOCATION '../../testing/data/csv/aggregate_test_100.csv'; | ||
|
||
query CC | ||
explain SELECT c1 FROM aggregate_test_100 where c2 > 10 |
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.
Here is an explain test in sqllogictest output!
4bb2615
to
cded193
Compare
@@ -673,42 +673,6 @@ async fn test_physical_plan_display_indent_multi_children() { | |||
); | |||
} | |||
|
|||
#[tokio::test] | |||
#[cfg_attr(tarpaulin, ignore)] | |||
async fn csv_explain() { |
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.
this test is ported to the sqllogictest framework as an example
]]; | ||
assert_eq!(expected, actual); | ||
|
||
let sql = "explain SELECT c1 FROM aggregate_test_100 where c2 > 10"; |
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.
this second test was to show that the explicit casting was removed but I don't think that is needed anymore with the improved coverage of casting as well as the (about to be) better plan test coverage
FYI @xudong963 and @melgenek |
Co-authored-by: Yevhenii Melnyk <melnyk.yevhenii@gmail.com>
WITH HEADER ROW | ||
LOCATION '../../testing/data/csv/aggregate_test_100.csv'; | ||
|
||
query ?? |
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 wonder if query ?
works?
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.
(it was added by --complete
mode, so I believe it does)
Benchmark runs are scheduled for baseline = 8a609dc and contender = 50f6e69. 50f6e69 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Part of #4495
Rationale for this change
As part of porting window functions tests in #5199 I found there was additional work to do for normalizing explain plans.
What changes are included in this PR?
Are these changes tested?
Yes
Are there any user-facing changes?
No