Conversation
github-actions
bot
added
core
| if let Ok(mut statements) = DFParser::parse_sql(&sql) { | ||
| statements.pop_front().expect("at least one SQL statement"); | ||
| } |
There was a problem hiding this comment.
This piece was needed to have workarounds for creating tables without values. It is not needed anymore.
| Ok(()) | ||
| pub fn main() { | ||
| // Tests from `tpch.slt` fail with stackoverflow with the default stack size. | ||
| thread::Builder::new() |
There was a problem hiding this comment.
tpch.slt test fails with stack overflow errors. The stack size on Windows seems to be 1Mb by default vs 2MB on Linux. I tried debugging what causes the error, but it seems that the stack is just deep enough due to the recursive design of the physical planner.
In case somebody with more understanding takes a closer look, I added a stack trace for the overflow as well as a test to reproduce the issue with unit tests on any platform here https://gist.github.com/melgenek/b6acf4467ee10c1941f3b103a17e1b1e.
There was a problem hiding this comment.
We have hit this before (e.g. #4065) -- I think cranking up the default size is fine.
Specifically, release builds seem to use much less stack so this is primarily a test issue rather than a production issue
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
LGTM -- thank you @melgenek
I also verified that the windows CI run indeed is running sqllogictests now: https://github.com/apache/arrow-datafusion/actions/runs/4611787296/jobs/8151915512?pr=5870#step:5:2877
| Ok(()) | ||
| pub fn main() { | ||
| // Tests from `tpch.slt` fail with stackoverflow with the default stack size. | ||
| thread::Builder::new() |
There was a problem hiding this comment.
We have hit this before (e.g. #4065) -- I think cranking up the default size is fine.
Specifically, release builds seem to use much less stack so this is primarily a test issue rather than a production issue
2 participants
Which issue does this PR close?
Closes #4494.
Rationale for this change
Run sqllogictest tests in Windows in Github actions. These tests are currently disabled.
What changes are included in this PR?
Enable sqllogictest tests in Windows in Github actions. Additionally, there are some tricks to make tests work.
Are these changes tested?
The change is a test only and runs in ci.
Are there any user-facing changes?
N/A