Remove unnecessary Box

[ever0de]

Previously, Query variant in enum Statement seemed to be used recursively, but this is not the case now.

[coveralls]

Pull Request Test Coverage Report for Build 2808289422

• 8 of 8 (100.0%) changed or added relevant lines in 4 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.001%) to 89.954%

---

Totals
Change from base Build 2805421159:	0.001%
Covered Lines:	9097
Relevant Lines:	10113

---

💛 - Coveralls

[alamb]

I believe this Box was added in #540 by @5tan to save stack space

Perhaps we can add a comment somewhere to the code explaining the rationale?

[ever0de]

I believe this Box was added in #540 by @5tan to save stack space

Perhaps we can add a comment somewhere to the code explaining the rationale?

There was this context. thank you
I'm a little confused, but using `Box' saves stack space, but doesn't it use that much heap space? I don't know why this is better.
What should I look for to understand this?

[alamb]

What should I look for to understand this?

Here is a description of the issue that I like (though I am obviously biased ): apache/datafusion#1047

It is for a slightly different issue, but the principle is the same

[andygrove]

Oops ... I approved before seeing the comments from @alamb

[ever0de]

Oops ... I approved before seeing the comments from @alamb

Yes, this should be a work in progress.
However, In my opinion it is still not clear whether the Query occupies a stack size large enough to be a 'Box'.
If you boxed because of that problem, It seems that I have some understanding...?

I am looking at the work @alamb linked.

[alamb]

Marking as draft as we work on this issue 👍 thanks @ever0de

[ever0de]

Marking as draft as we work on this issue 👍 thanks @ever0de

thank you I was wondering if I should change it.

[ever0de]

hello. At the current main 72559e9 commit, I tried running the sql syntax of #540 PR, but it fails with stack overflow. Maybe I've missed something?

Using Command

x=286 ; echo 'print("SELECT * FROM {}ttt{}".format("("*'$x',")"*'$x'))' | python3 > test.sql && cargo run --example cli --manifest-path Cargo.toml test.sql --snowflake

Output

Parsing from file 'test.sql' using SnowflakeDialect
2022-08-26T06:59:00.199Z DEBUG [sqlparser::parser] Parsing sql 'SELECT * FROM ((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((ttt))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))
'...

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
[1]    11226 abort      cargo run --example cli --manifest-path Cargo.toml test.sql --snowflake

[5tan]

Hi friends! Currently size_of::<Query>()=592. If we unbox the query body back (revert my changes), then size_of::<Query>()=1360.

[andygrove]

Perhaps we can add a comment somewhere to the code explaining the rationale?

@ever0de Would you mind updating this PR to revert the changes but add a comment instead?

[ever0de]

Perhaps we can add a comment somewhere to the code explaining the rationale?

@ever0de Would you mind updating this PR to revert the changes but add a comment instead?

First of all, I understand that this PR is a bad fix.
I will close the PR first, and then bring it with me if there is an improvement plan.
Thanks for the good information.

Can I close this PR?

[andygrove]

Can I close this PR?

Yes, please go ahead! Thanks.