Skip to content
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

datafusion-cli: add streaming state for printing logic #14961

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shruti2522
Copy link

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?

  • added OutputStream struct encapsulating all the mut variables
  • additional tests

Are these changes tested?

Yes

Are there any user-facing changes?

@shruti2522
Copy link
Author

Hi @alamb, please trigger the CI for this.

@shruti2522
Copy link
Author

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?

@zhuqi-lucas
Copy link
Contributor

zhuqi-lucas commented Mar 3, 2025

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:
#14953

And you may need to rebase after it merged, i will help review this PR after it, thanks!

@@ -153,6 +155,164 @@ fn format_batches_with_maxrows<W: std::io::Write>(
Ok(())
}

/// The state and methods for displaying output
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code clean for new datafusion-cli streaming printing logic
3 participants