Skip to content

Commit

Permalink
Add Display for Expr::BinaryExpr
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@matthewmturner
matthewmturner committed Sep 21, 2021
1 parent ddbfc74 commit c856388
Show file tree
Hide file tree
Showing 13 changed files with 203 additions and 183 deletions.
18 changes: 9 additions & 9 deletions datafusion/src/execution/context.rs
View file
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
38 changes: 29 additions & 9 deletions datafusion/src/logical_plan/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,20 @@ impl std::fmt::Display for Expr {
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),
}
}
Expand Down Expand Up @@ -1585,8 +1599,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 @@ -1649,10 +1669,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 @@ -1661,7 +1681,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 @@ -1684,9 +1704,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 @@ -1744,7 +1764,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 @@ -1892,12 +1912,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
10 changes: 5 additions & 5 deletions datafusion/src/logical_plan/plan.rs
View file
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Loading

0 comments on commit c856388

Please sign in to comment.