From 251260849ebee7d5d87f8091a70b4d11ca5ae91a Mon Sep 17 00:00:00 2001
From: "xudong.w" <wxd963996380@gmail.com>
Date: Mon, 31 Jan 2022 22:45:14 +0800
Subject: [PATCH] add annotation for select_to_plan (#1714)

---
 datafusion/src/sql/planner.rs | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/datafusion/src/sql/planner.rs b/datafusion/src/sql/planner.rs
index 9da54cad4daa..a74c44665de1 100644
--- a/datafusion/src/sql/planner.rs
+++ b/datafusion/src/sql/planner.rs
@@ -697,14 +697,13 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
         }
     }
 
-    /// Generate a logic plan from an SQL select
-    fn select_to_plan(
+    /// Generate a logic plan from selection clause, the function contain optimization for cross join to inner join
+    /// Related PR: https://github.com/apache/arrow-datafusion/pull/1566
+    fn plan_selection(
         &self,
         select: &Select,
-        ctes: &mut HashMap<String, LogicalPlan>,
-        alias: Option<String>,
+        plans: Vec<LogicalPlan>,
     ) -> Result<LogicalPlan> {
-        let plans = self.plan_from_tables(&select.from, ctes)?;
         let plan = match &select.selection {
             Some(predicate_expr) => {
                 // build join schema
@@ -822,9 +821,23 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
                 }
             }
         };
-        let plan = plan?;
+        plan
+    }
 
-        // The SELECT expressions, with wildcards expanded.
+    /// Generate a logic plan from an SQL select
+    fn select_to_plan(
+        &self,
+        select: &Select,
+        ctes: &mut HashMap<String, LogicalPlan>,
+        alias: Option<String>,
+    ) -> Result<LogicalPlan> {
+        // process `from` clause
+        let plans = self.plan_from_tables(&select.from, ctes)?;
+
+        // process `where` clause
+        let plan = self.plan_selection(select, plans)?;
+
+        // process the SELECT expressions, with wildcards expanded.
         let select_exprs = self.prepare_select_exprs(&plan, select)?;
 
         // having and group by clause may reference aliases defined in select projection
@@ -873,6 +886,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
         // All of the aggregate expressions (deduplicated).
         let aggr_exprs = find_aggregate_exprs(&aggr_expr_haystack);
 
+        // All of the group by expressions
         let group_by_exprs = select
             .group_by
             .iter()
@@ -891,6 +905,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
             })
             .collect::<Result<Vec<Expr>>>()?;
 
+        // process group by, aggregation or having
         let (plan, select_exprs_post_aggr, having_expr_post_aggr_opt) = if !group_by_exprs
             .is_empty()
             || !aggr_exprs.is_empty()
@@ -931,7 +946,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
             plan
         };
 
-        // window function
+        // process window function
         let window_func_exprs = find_window_exprs(&select_exprs_post_aggr);
 
         let plan = if window_func_exprs.is_empty() {
@@ -940,6 +955,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
             LogicalPlanBuilder::window_plan(plan, window_func_exprs)?
         };
 
+        // process distinct clause
         let plan = if select.distinct {
             return LogicalPlanBuilder::from(plan)
                 .aggregate(select_exprs_post_aggr, iter::empty::<Expr>())?
@@ -947,6 +963,8 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
         } else {
             plan
         };
+
+        // generate the final projection plan
         project_with_alias(plan, select_exprs_post_aggr, alias)
     }