Skip to content

Commit

Permalink
Implement Display for Expr, improve operator display (#971)
Browse files Browse the repository at this point in the history 
* # This is a combination of 3 commits.
# This is the 1st commit message:

Add Display for Expr::BinaryExpr

# This is the commit message #2:

Update logical_plan/operators tests

# This is the commit message #3:

rebase and debug display for non binary expr

* Add Display for Expr::BinaryExpr

Update logical_plan/operators tests

rebase and debug display for non binary expr

Add Display for Expr::BinaryExpr

Update logical_plan/operators tests

Updating tests

Update aggregate display

Updating tests without aggregate

More tests

Working on agg/scalar functions

Fix binary_expr in create_name function and attendant tests

More tests

More tests

Doc tests

Rebase and update new tests

* Submodule update

* Restore submodule references from master

Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
  • Loading branch information
@matthewmturner @alamb
matthewmturner and alamb committed Sep 22, 2021
1 parent 1c858ce commit a02d5e1
Show file tree
Hide file tree
Showing 13 changed files with 220 additions and 187 deletions.
18 changes: 9 additions & 9 deletions datafusion/src/execution/context.rs
View file
Expand Up @@ -1696,15 +1696,15 @@ mod tests {
.await?;

let expected = vec![
"+----+----+--------------+-----------------------------------+----------------------------------+------------------------------------------+--------------+----------------+--------------+--------------+--------------+",
"| c1 | c2 | ROW_NUMBER() | FIRST_VALUE(test.c2 Plus test.c1) | LAST_VALUE(test.c2 Plus test.c1) | NTH_VALUE(test.c2 Plus test.c1,Int64(1)) | SUM(test.c2) | COUNT(test.c2) | MAX(test.c2) | MIN(test.c2) | AVG(test.c2) |",
"+----+----+--------------+-----------------------------------+----------------------------------+------------------------------------------+--------------+----------------+--------------+--------------+--------------+",
"| 0 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 |",
"| 0 | 2 | 1 | 2 | 2 | 2 | 2 | 1 | 2 | 2 | 2 |",
"| 0 | 3 | 1 | 3 | 3 | 3 | 3 | 1 | 3 | 3 | 3 |",
"| 0 | 4 | 1 | 4 | 4 | 4 | 4 | 1 | 4 | 4 | 4 |",
"| 0 | 5 | 1 | 5 | 5 | 5 | 5 | 1 | 5 | 5 | 5 |",
"+----+----+--------------+-----------------------------------+----------------------------------+------------------------------------------+--------------+----------------+--------------+--------------+--------------+",
"+----+----+--------------+--------------------------------+-------------------------------+---------------------------------------+--------------+----------------+--------------+--------------+--------------+",
"| c1 | c2 | ROW_NUMBER() | FIRST_VALUE(test.c2 + test.c1) | LAST_VALUE(test.c2 + test.c1) | NTH_VALUE(test.c2 + test.c1,Int64(1)) | SUM(test.c2) | COUNT(test.c2) | MAX(test.c2) | MIN(test.c2) | AVG(test.c2) |",
"+----+----+--------------+--------------------------------+-------------------------------+---------------------------------------+--------------+----------------+--------------+--------------+--------------+",
"| 0 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 | 1 |",
"| 0 | 2 | 1 | 2 | 2 | 2 | 2 | 1 | 2 | 2 | 2 |",
"| 0 | 3 | 1 | 3 | 3 | 3 | 3 | 1 | 3 | 3 | 3 |",
"| 0 | 4 | 1 | 4 | 4 | 4 | 4 | 1 | 4 | 4 | 4 |",
"| 0 | 5 | 1 | 5 | 5 | 5 | 5 | 1 | 5 | 5 | 5 |",
"+----+----+--------------+--------------------------------+-------------------------------+---------------------------------------+--------------+----------------+--------------+--------------+--------------+",
];

// window function shall respect ordering
Expand Down
2 changes: 1 addition & 1 deletion datafusion/src/logical_plan/builder.rs
View file
Expand Up @@ -663,7 +663,7 @@ mod tests {
.build()?;

let expected = "Projection: #employee_csv.id\
\n Filter: #employee_csv.state Eq Utf8(\"CO\")\
\n Filter: #employee_csv.state = Utf8(\"CO\")\
\n TableScan: employee_csv projection=Some([0, 3])";

assert_eq!(expected, format!("{:?}", plan));
Expand Down
53 changes: 43 additions & 10 deletions datafusion/src/logical_plan/expr.rs
View file
Expand Up @@ -941,6 +941,33 @@ impl Not for Expr {
}
}

impl std::fmt::Display for Expr {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Expr::BinaryExpr {
ref left,
ref right,
ref op,
} => write!(f, "{} {} {}", left, op, right),
Expr::AggregateFunction {
/// Name of the function
ref fun,
/// List of expressions to feed to the functions as arguments
ref args,
/// Whether this is a DISTINCT aggregation or not
ref distinct,
} => fmt_function(f, &fun.to_string(), *distinct, args, true),
Expr::ScalarFunction {
/// Name of the function
ref fun,
/// List of expressions to feed to the functions as arguments
ref args,
} => fmt_function(f, &fun.to_string(), false, args, true),
_ => write!(f, "{:?}", self),
}
}
}

#[allow(clippy::boxed_local)]
fn rewrite_boxed<R>(boxed_expr: Box<Expr>, rewriter: &mut R) -> Result<Box<Expr>>
where
Expand Down Expand Up @@ -1603,8 +1630,14 @@ fn fmt_function(
fun: &str,
distinct: bool,
args: &[Expr],
display: bool,
) -> fmt::Result {
let args: Vec<String> = args.iter().map(|arg| format!("{:?}", arg)).collect();
let args: Vec<String> = match display {
true => args.iter().map(|arg| format!("{}", arg)).collect(),
false => args.iter().map(|arg| format!("{:?}", arg)).collect(),
};

// let args: Vec<String> = args.iter().map(|arg| format!("{:?}", arg)).collect();
let distinct_str = match distinct {
true => "DISTINCT ",
false => "",
Expand Down Expand Up @@ -1648,7 +1681,7 @@ impl fmt::Debug for Expr {
Expr::IsNull(expr) => write!(f, "{:?} IS NULL", expr),
Expr::IsNotNull(expr) => write!(f, "{:?} IS NOT NULL", expr),
Expr::BinaryExpr { left, op, right } => {
write!(f, "{:?} {:?} {:?}", left, op, right)
write!(f, "{:?} {} {:?}", left, op, right)
}
Expr::Sort {
expr,
Expand All @@ -1667,10 +1700,10 @@ impl fmt::Debug for Expr {
}
}
Expr::ScalarFunction { fun, args, .. } => {
fmt_function(f, &fun.to_string(), false, args)
fmt_function(f, &fun.to_string(), false, args, false)
}
Expr::ScalarUDF { fun, ref args, .. } => {
fmt_function(f, &fun.name, false, args)
fmt_function(f, &fun.name, false, args, false)
}
Expr::WindowFunction {
fun,
Expand All @@ -1679,7 +1712,7 @@ impl fmt::Debug for Expr {
order_by,
window_frame,
} => {
fmt_function(f, &fun.to_string(), false, args)?;
fmt_function(f, &fun.to_string(), false, args, false)?;
if !partition_by.is_empty() {
write!(f, " PARTITION BY {:?}", partition_by)?;
}
Expand All @@ -1702,9 +1735,9 @@ impl fmt::Debug for Expr {
distinct,
ref args,
..
} => fmt_function(f, &fun.to_string(), *distinct, args),
} => fmt_function(f, &fun.to_string(), *distinct, args, true),
Expr::AggregateUDF { fun, ref args, .. } => {
fmt_function(f, &fun.name, false, args)
fmt_function(f, &fun.name, false, args, false)
}
Expr::Between {
expr,
Expand Down Expand Up @@ -1762,7 +1795,7 @@ fn create_name(e: &Expr, input_schema: &DFSchema) -> Result<String> {
Expr::BinaryExpr { left, op, right } => {
let left = create_name(left, input_schema)?;
let right = create_name(right, input_schema)?;
Ok(format!("{} {:?} {}", left, op, right))
Ok(format!("{} {} {}", left, op, right))
}
Expr::Case {
expr,
Expand Down Expand Up @@ -1925,12 +1958,12 @@ mod tests {
assert_eq!(
rewriter.v,
vec![
"Previsited #state Eq Utf8(\"CO\")",
"Previsited #state = Utf8(\"CO\")",
"Previsited #state",
"Mutated #state",
"Previsited Utf8(\"CO\")",
"Mutated Utf8(\"CO\")",
"Mutated #state Eq Utf8(\"CO\")"
"Mutated #state = Utf8(\"CO\")"
]
)
}
Expand Down
8 changes: 4 additions & 4 deletions datafusion/src/logical_plan/operators.rs
View file
Expand Up @@ -129,19 +129,19 @@ mod tests {
fn test_operators() {
assert_eq!(
format!("{:?}", lit(1u32) + lit(2u32)),
"UInt32(1) Plus UInt32(2)"
"UInt32(1) + UInt32(2)"
);
assert_eq!(
format!("{:?}", lit(1u32) - lit(2u32)),
"UInt32(1) Minus UInt32(2)"
"UInt32(1) - UInt32(2)"
);
assert_eq!(
format!("{:?}", lit(1u32) * lit(2u32)),
"UInt32(1) Multiply UInt32(2)"
"UInt32(1) * UInt32(2)"
);
assert_eq!(
format!("{:?}", lit(1u32) / lit(2u32)),
"UInt32(1) Divide UInt32(2)"
"UInt32(1) / UInt32(2)"
);
}
}
10 changes: 5 additions & 5 deletions datafusion/src/logical_plan/plan.rs
View file
Expand Up @@ -551,7 +551,7 @@ impl LogicalPlan {
/// // Format using display_indent
/// let display_string = format!("{}", plan.display_indent());
///
/// assert_eq!("Filter: #foo_csv.id Eq Int32(5)\
/// assert_eq!("Filter: #foo_csv.id = Int32(5)\
/// \n TableScan: foo_csv projection=None",
/// display_string);
/// ```
Expand All @@ -575,7 +575,7 @@ impl LogicalPlan {
///
/// ```text
/// Projection: #employee.id [id:Int32]\
/// Filter: #employee.state Eq Utf8(\"CO\") [id:Int32, state:Utf8]\
/// Filter: #employee.state = Utf8(\"CO\") [id:Int32, state:Utf8]\
/// TableScan: employee projection=Some([0, 3]) [id:Int32, state:Utf8]";
/// ```
///
Expand All @@ -592,7 +592,7 @@ impl LogicalPlan {
/// // Format using display_indent_schema
/// let display_string = format!("{}", plan.display_indent_schema());
///
/// assert_eq!("Filter: #foo_csv.id Eq Int32(5) [id:Int32]\
/// assert_eq!("Filter: #foo_csv.id = Int32(5) [id:Int32]\
/// \n TableScan: foo_csv projection=None [id:Int32]",
/// display_string);
/// ```
Expand Down Expand Up @@ -939,7 +939,7 @@ mod tests {
let plan = display_plan();

let expected = "Projection: #employee_csv.id\
\n Filter: #employee_csv.state Eq Utf8(\"CO\")\
\n Filter: #employee_csv.state = Utf8(\"CO\")\
\n TableScan: employee_csv projection=Some([0, 3])";

assert_eq!(expected, format!("{}", plan.display_indent()));
Expand All @@ -950,7 +950,7 @@ mod tests {
let plan = display_plan();

let expected = "Projection: #employee_csv.id [id:Int32]\
\n Filter: #employee_csv.state Eq Utf8(\"CO\") [id:Int32, state:Utf8]\
\n Filter: #employee_csv.state = Utf8(\"CO\") [id:Int32, state:Utf8]\
\n TableScan: employee_csv projection=Some([0, 3]) [id:Int32, state:Utf8]";

assert_eq!(expected, format!("{}", plan.display_indent_schema()));
Expand Down
16 changes: 8 additions & 8 deletions datafusion/src/optimizer/common_subexpr_eliminate.rs
View file
Expand Up @@ -714,8 +714,8 @@ mod test {
)?
.build()?;

let expected = "Aggregate: groupBy=[[]], aggr=[[SUM(#BinaryExpr-*BinaryExpr--Column-test.bLiteral1Column-test.a AS test.a Multiply Int32(1) Minus test.b), SUM(#BinaryExpr-*BinaryExpr--Column-test.bLiteral1Column-test.a AS test.a Multiply Int32(1) Minus test.b Multiply Int32(1) Plus #test.c)]]\
\n Projection: #test.a Multiply Int32(1) Minus #test.b AS BinaryExpr-*BinaryExpr--Column-test.bLiteral1Column-test.a, #test.a, #test.b, #test.c\
let expected = "Aggregate: groupBy=[[]], aggr=[[SUM(#BinaryExpr-*BinaryExpr--Column-test.bLiteral1Column-test.a AS test.a * Int32(1) - test.b), SUM(#BinaryExpr-*BinaryExpr--Column-test.bLiteral1Column-test.a AS test.a * Int32(1) - test.b * Int32(1) + #test.c)]]\
\n Projection: #test.a * Int32(1) - #test.b AS BinaryExpr-*BinaryExpr--Column-test.bLiteral1Column-test.a, #test.a, #test.b, #test.c\
\n TableScan: test projection=None";

assert_optimized_plan_eq(&plan, expected);
Expand All @@ -737,7 +737,7 @@ mod test {
)?
.build()?;

let expected = "Aggregate: groupBy=[[]], aggr=[[Int32(1) Plus #AggregateFunction-AVGfalseColumn-test.a AS AVG(test.a), Int32(1) Minus #AggregateFunction-AVGfalseColumn-test.a AS AVG(test.a)]]\
let expected = "Aggregate: groupBy=[[]], aggr=[[Int32(1) + #AggregateFunction-AVGfalseColumn-test.a AS AVG(test.a), Int32(1) - #AggregateFunction-AVGfalseColumn-test.a AS AVG(test.a)]]\
\n Projection: AVG(#test.a) AS AggregateFunction-AVGfalseColumn-test.a, #test.a, #test.b, #test.c\
\n TableScan: test projection=None";

Expand All @@ -757,8 +757,8 @@ mod test {
])?
.build()?;

let expected = "Projection: #BinaryExpr-+Column-test.aLiteral1 AS Int32(1) Plus test.a AS first, #BinaryExpr-+Column-test.aLiteral1 AS Int32(1) Plus test.a AS second\
\n Projection: Int32(1) Plus #test.a AS BinaryExpr-+Column-test.aLiteral1, #test.a, #test.b, #test.c\
let expected = "Projection: #BinaryExpr-+Column-test.aLiteral1 AS Int32(1) + test.a AS first, #BinaryExpr-+Column-test.aLiteral1 AS Int32(1) + test.a AS second\
\n Projection: Int32(1) + #test.a AS BinaryExpr-+Column-test.aLiteral1, #test.a, #test.b, #test.c\
\n TableScan: test projection=None";

assert_optimized_plan_eq(&plan, expected);
Expand All @@ -777,7 +777,7 @@ mod test {
])?
.build()?;

let expected = "Projection: Int32(1) Plus #test.a, #test.a Plus Int32(1)\
let expected = "Projection: Int32(1) + #test.a, #test.a + Int32(1)\
\n TableScan: test projection=None";

assert_optimized_plan_eq(&plan, expected);
Expand All @@ -794,8 +794,8 @@ mod test {
.project(vec![binary_expr(lit(1), Operator::Plus, col("a"))])?
.build()?;

let expected = "Projection: #Int32(1) Plus test.a\
\n Projection: Int32(1) Plus #test.a\
let expected = "Projection: #Int32(1) + test.a\
\n Projection: Int32(1) + #test.a\
\n TableScan: test projection=None";

assert_optimized_plan_eq(&plan, expected);
Expand Down
4 changes: 2 additions & 2 deletions datafusion/src/optimizer/constant_folding.rs
View file
Expand Up @@ -592,7 +592,7 @@ mod tests {

let expected = "\
Projection: #test.a\
\n Filter: NOT #test.b And #test.c\
\n Filter: NOT #test.b AND #test.c\
\n TableScan: test projection=None";

assert_optimized_plan_eq(&plan, expected);
Expand All @@ -609,7 +609,7 @@ mod tests {

let expected = "\
Projection: #test.a\
\n Filter: NOT #test.b Or NOT #test.c\
\n Filter: NOT #test.b OR NOT #test.c\
\n TableScan: test projection=None";

assert_optimized_plan_eq(&plan, expected);
Expand Down

0 comments on commit a02d5e1

Please sign in to comment.