-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat: Better large output display in datafusion-cli with --maxrows option #7617
Conversation
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 @2010YOUY01 -- this is great. I tried it out locally and the code looks good to me.
I think we could encapsulate the code a bit and make a MaxRows
type structure like this that would make parsing a little less awkward:
struct Args {
...
#[clap(
long,
help = "The max number of rows to display for 'Table' format\n[default: 40] [possible values: numbers(0/10/...), inf(no limit)]",
default_value = "40",
)]
maxrows: MaxRows,
}
With a structure like
#[derive(Debug, Clone)]
enum MaxRows {
/// show all rows in the output
Unlimited,
/// Only show n rows
Limited(usize)
}
impl FromStr for MaxRows {
type Err = String;
fn from_str(maxrows: &str) -> Result<Self, Self::Err> {
if maxrows.to_lowercase() == "inf"
|| maxrows.to_lowercase() == "infinite"
|| maxrows.to_lowercase() == "none"
{
Ok(Self::Unlimited)
} else {
match maxrows.parse::<usize>() {
Ok(nrows) => Ok(Self::Limited(nrows)),
_ => Err(format!("Invalid maxrows {}. Valid inputs are natural numbers or \'inf\' for no limit.", maxrows)),
}
}
}
}
impl Display for MaxRows {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::Unlimited => write!(f, "unlimited"),
Self::Limited(max_rows) => write!(f, "at most max_rows"),
}
}
}
datafusion-cli/src/main.rs
Outdated
} else { | ||
match maxrows.parse::<usize>() { | ||
Ok(nrows) => Ok(Some(nrows)), | ||
_ => Err(format!("Invalid maxrows {}. Valid inputs are natural numbers or \'inf\' for no limit.", maxrows)), |
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.
_ => Err(format!("Invalid maxrows {}. Valid inputs are natural numbers or \'inf\' for no limit.", maxrows)), | |
_ => Err(format!("Invalid maxrows {}. Valid inputs are natural numbers or \'none\', \'inf\', or \'infinite\' for no limit.", maxrows)), |
Thank you for the suggestions |
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.
Looks great -- thank you @2010YOUY01
@@ -211,6 +211,15 @@ async fn exec_and_print( | |||
let statements = DFParser::parse_sql_with_dialect(&sql, dialect.as_ref())?; | |||
for statement in statements { | |||
let plan = ctx.state().statement_to_plan(statement).await?; | |||
|
|||
// For plans like `Explain` ignore `MaxRows` option and always display all rows |
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.
👍
…tion (apache#7617) * Better large output display for CLI * review comments
Which issue does this PR close?
NA
Rationale for this change
If a query outputs a large number of lines, currently datafusion-cli will display them all and flood the terminal.
This PR added a
--maxrows
option to limit max number of rows for output display.What changes are included in this PR?
[RecordBatch]
), before it also counts the time for formatting and printing the results.--maxrows
option, now onlyTable
CLI display format is supported.Are these changes tested?
Unit tests
Are there any user-facing changes?
No