datafusion-cli: add streaming state for printing logic #14961
Conversation
|
Hi @alamb, please trigger the CI for this. |
|
Hi @alamb , @zhuqi-lucas, I see that the most of the functions in the datafusion cli print format have been reverted, do we still need these changes? |
|
Thank you @shruti2522 for the work, we still need this feature, we revert it due to urgent release cycle for the new feature testing and bug fixes, actually we will add back the feature in this PR: And you may need to rebase after it merged, i will help review this PR after it, thanks! |
datafusion-cli/src/print_format.rs
Outdated
| @@ -153,6 +155,164 @@ fn format_batches_with_maxrows<W: std::io::Write>( | |||
| Ok(()) | |||
| } | |||
|
|
|||
| /// The state and methods for displaying output | |||
There was a problem hiding this comment.
This definitely looks like it is on the right track -- thank you @shruti2522
Can you perhaps try and migrate the old callsites to use this new code?
As @zhuqi-lucas mentions, there is another version of the PR here:
I am hoping we can get the structure in #14954 cleaner before merging. Maybe you can help there
There was a problem hiding this comment.
@shruti2522 May be after #14954 finalizing, you can open a PR to my branch, so we can review and merge then? @alamb I am also wandering how we can collaborate the work? Thanks!
|
Marking as draft as I think this PR is no longer waiting on feedback. Please mark it as ready for review when it is ready for another look |
|
Thanks @alamb, @zhuqi-lucas, I’ll rebase after PR #14954 is merged. |
Which issue does this PR close?
Rationale for this change
code clean for new datafusion-cli printing logic
What changes are included in this PR?
OutputStreamstruct encapsulating all the mut variablesAre these changes tested?
Yes
Are there any user-facing changes?