-
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 more window tests to sqlogictests #5434
Conversation
|
||
Ok(()) | ||
} | ||
|
||
fn write_test_data_to_parquet(tmpdir: &TempDir, n_file: usize) -> Result<()> { |
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 file has only a few hundred lines after this one 😅
There appear to be failing tests? |
Looks like some non deterministic output (a LIMIT without an order by) -- fixed in debdf41 |
@@ -92,7 +92,7 @@ limit 5 | |||
|
|||
|
|||
|
|||
# async fn csv_query_window_with_order_by |
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 thought it was left on purpose in the past😂
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.
If you think it adds value I can put them back in 😆 -- I think it was just left over from incomplete porting
Benchmark runs are scheduled for baseline = f68214d and contender = a95e0ec. a95e0ec 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
Follow on to #5330
Rationale for this change
I am trying to keep the testability of DataFusion reasonable. Sqllogictests are easier to add / update so we are trying to move stuff over there.
What changes are included in this PR?
Port more window tests to window.slt
Are these changes tested?
Yes all tests
Are there any user-facing changes?
No