NGA-TRAN commented on a change in pull request #858:
URL: https://github.com/apache/arrow-datafusion/pull/858#discussion_r687809683
##########
File path: ballista/rust/core/src/serde/logical_plan/mod.rs
##########
@@ -661,6 +661,43 @@ mod roundtrip_tests {
Ok(())
}
+ #[test]
+ fn roundtrip_analyze() -> Result<()> {
+ let schema = Schema::new(vec![
+ Field::new("id", DataType::Int32, false),
+ Field::new("first_name", DataType::Utf8, false),
+ Field::new("last_name", DataType::Utf8, false),
+ Field::new("state", DataType::Utf8, false),
+ Field::new("salary", DataType::Int32, false),
+ ]);
+
+ let verbose_plan = LogicalPlanBuilder::scan_csv(
+ "employee.csv",
+ CsvReadOptions::new().schema(&schema).has_header(true),
+ Some(vec![3, 4]),
+ )
+ .and_then(|plan| plan.sort(vec![col("salary")]))
+ .and_then(|plan| plan.explain(true, true))
+ .and_then(|plan| plan.build())
+ .map_err(BallistaError::DataFusionError)?;
+
+ let plan = LogicalPlanBuilder::scan_csv(
+ "employee.csv",
+ CsvReadOptions::new().schema(&schema).has_header(true),
+ Some(vec![3, 4]),
+ )
+ .and_then(|plan| plan.sort(vec![col("salary")]))
+ .and_then(|plan| plan.explain(false, true))
+ .and_then(|plan| plan.build())
+ .map_err(BallistaError::DataFusionError)?;
+
+ roundtrip_test!(plan);
Review comment:
When we talk I need to learn the the effectiveness of this round trip
test thats convert a logical/physical plan into photo and back. The tests look
simple and easy to understand this way.
##########
File path: datafusion/src/logical_plan/plan.rs
##########
@@ -213,6 +213,16 @@ pub enum LogicalPlan {
/// The output schema of the explain (2 columns of text)
schema: DFSchemaRef,
},
+ /// Runs the actual plan, and then prints the physical plan with
+ /// with execution metrics.
+ Analyze {
Review comment:
I am about to ask when I saw the above code that has analyze as
different function. I am worry about future inconsistency and headache of
keeping them consistent, as well as redundant work when we change or improve
something. I would prefer to keep them in the same LogicalPlan
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
