You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/08/14 04:35:20 UTC
[GitHub] [arrow] jorgecarleitao commented on a change in pull request #7959: ARROW-9654: [Rust][DataFusion] Add `EXPLAIN ` statement
jorgecarleitao commented on a change in pull request #7959:
URL: https://github.com/apache/arrow/pull/7959#discussion_r470397152
##########
File path: rust/datafusion/tests/sql.rs
##########
@@ -432,6 +432,40 @@ fn csv_query_count_one() {
assert_eq!(expected, actual);
}
+#[test]
+fn csv_explain() {
+ let mut ctx = ExecutionContext::new();
+ register_aggregate_csv_by_sql(&mut ctx);
+ let sql = "EXPLAIN SELECT c1 FROM aggregate_test_100 where c2 > 10";
+ let actual = execute(&mut ctx, sql).join("\n");
+ let expected = "\"logical_plan\"\t\"Projection: #c1\\n Selection: #c2 Gt Int64(10)\\n TableScan: aggregate_test_100 projection=None\"".to_string();
+ assert_eq!(expected, actual);
+
+ // Also, expect same result with case explain
Review comment:
`with lowercase explain`?
##########
File path: rust/datafusion/src/logicalplan.rs
##########
@@ -1128,6 +1153,32 @@ impl LogicalPlanBuilder {
}
}
+/// Represents some sort of execution plan, in String form
+#[derive(Debug, Clone, PartialEq)]
+pub struct StringifiedPlan {
+ /// An identifier of what type of plan this string represents
+ pub plan_type: Arc<String>, // TODO make this an enum?
Review comment:
I would say so, this is a finite and well defined number of cases.
##########
File path: rust/datafusion/src/logicalplan.rs
##########
@@ -1128,6 +1153,32 @@ impl LogicalPlanBuilder {
}
}
+/// Represents some sort of execution plan, in String form
+#[derive(Debug, Clone, PartialEq)]
+pub struct StringifiedPlan {
+ /// An identifier of what type of plan this string represents
+ pub plan_type: Arc<String>, // TODO make this an enum?
+ /// The string representation of the plan
+ pub plan: Arc<String>,
+}
+
+impl StringifiedPlan {
+ /// Create a new Stringified plan of `plan_type` with string
+ /// representation `plan`
+ pub fn new(plan_type: impl Into<String>, plan: impl Into<String>) -> Self {
+ StringifiedPlan {
+ plan_type: Arc::new(plan_type.into()),
+ plan: Arc::new(plan.into()),
+ }
+ }
+
+ /// returns true if this plan should be displayed. Generally
+ /// `verbose_mode = true` will display all available plans
+ pub fn should_display(&self, verbose_mode: bool) -> bool {
Review comment:
I would try to be systematic with the name: if `LogicalPlan::Explain::verbose`, then `verbose_mode` here or vice-versa.
##########
File path: rust/datafusion/tests/sql.rs
##########
@@ -432,6 +432,40 @@ fn csv_query_count_one() {
assert_eq!(expected, actual);
}
+#[test]
+fn csv_explain() {
+ let mut ctx = ExecutionContext::new();
+ register_aggregate_csv_by_sql(&mut ctx);
+ let sql = "EXPLAIN SELECT c1 FROM aggregate_test_100 where c2 > 10";
+ let actual = execute(&mut ctx, sql).join("\n");
+ let expected = "\"logical_plan\"\t\"Projection: #c1\\n Selection: #c2 Gt Int64(10)\\n TableScan: aggregate_test_100 projection=None\"".to_string();
+ assert_eq!(expected, actual);
+
+ // Also, expect same result with case explain
+ let sql = "explain SELECT c1 FROM aggregate_test_100 where c2 > 10";
+ let actual = execute(&mut ctx, sql).join("\n");
+ assert_eq!(expected, actual);
+}
+
+#[test]
+fn csv_explain_verbose() {
+ let mut ctx = ExecutionContext::new();
+ register_aggregate_csv_by_sql(&mut ctx);
+ let sql = "EXPLAIN VERBOSE SELECT c1 FROM aggregate_test_100 where c2 > 10";
+ let actual = execute(&mut ctx, sql).join("\n");
+ // Don't actually test the contents of the debuging output (as
Review comment:
yes! :)
##########
File path: rust/datafusion/src/optimizer/projection_push_down.rs
##########
@@ -45,7 +50,7 @@ impl ProjectionPushDown {
}
fn optimize_plan(
- &self,
+ &mut self,
Review comment:
IMO that is fine, these optimizers are just implementation structs anyway, so changing state has no impact.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org