-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: Improve datafusion-cli memory usage and considering reserve mem… #14766
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
Changes from 5 commits
69412e9
54bc8b7
ac7e783
ac24222
31866e9
1f9026c
b7205fd
0ecc9d2
aa838b2
4380843
a0a0db1
6715d23
ef4f184
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,6 +121,13 @@ struct Args { | |
| )] | ||
| maxrows: MaxRows, | ||
|
|
||
| #[clap( | ||
| short, | ||
| long, | ||
| help = "Whether to stop early when max rows is reached, this will help reduce the memory usage when the result is large" | ||
|
||
| )] | ||
| stop_after_max_rows: bool, | ||
|
|
||
| #[clap(long, help = "Enables console syntax highlighting")] | ||
| color: bool, | ||
| } | ||
|
|
@@ -186,6 +193,7 @@ async fn main_inner() -> Result<()> { | |
| quiet: args.quiet, | ||
| maxrows: args.maxrows, | ||
| color: args.color, | ||
| stop_after_max_rows: args.stop_after_max_rows, | ||
| }; | ||
|
|
||
| let commands = args.command; | ||
|
|
||
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 break here it will stop the plan early -- then you need to add
stop_after_max_rowsto permit the plan to run to completion, however, if that option is set then the data is buffered again (which from a memory perspective is the same as increasing the max_rows)Rather than
breaking here, I recommend just ignoring the batches aftermax_rowshas been reached (aka don'tpushthem intoresults) -- I think that would be the most intuitive behavior: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.
Thank you @alamb for review and the great idea! It makes sense to me, addressed in latest PR.