[SPARK-7232] [SQL] Add a Substitution batch for spark sql analyzer #5776

scwf · 2015-04-29T10:18:19Z

Added a new batch named Substitution before Resolution batch. The motivation for this is there are kind of cases we want to do some substitution on the parsed logical plan before resolve it.
Consider this two cases:
1 CTE, for cte we first build a row logical plan

'With Map(q1 -> 'Subquery q1
                   'Project ['key]
                      'UnresolvedRelation [src], None)
 'Project [*]
  'Filter ('key = 5)
   'UnresolvedRelation [q1], None

In With logicalplan here is a map stored the (q1-> subquery), we want first take off the with command and substitute the q1 of UnresolvedRelation by the subquery

2 Another example is Window function, in window function user may define some windows, we also need substitute the window name of child by the concrete window. this should also done in the Substitution batch.

AmplabJenkins · 2015-04-29T10:22:10Z

Merged build triggered.

AmplabJenkins · 2015-04-29T10:22:18Z

Merged build started.

SparkQA · 2015-04-29T10:22:49Z

Test build #31275 has started for PR 5776 at commit 29aaaaf.

SparkQA · 2015-04-29T13:13:58Z

Test build #31275 timed out for PR 5776 at commit 29aaaaf after a configured wait of 150m.

AmplabJenkins · 2015-04-29T13:14:02Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-04-29T13:14:02Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31275/
Test FAILed.

scwf · 2015-04-29T14:41:01Z

Retest this please

AmplabJenkins · 2015-04-29T14:42:11Z

Merged build triggered.

AmplabJenkins · 2015-04-29T14:42:20Z

Merged build started.

SparkQA · 2015-04-29T14:43:02Z

Test build #31286 has started for PR 5776 at commit 29aaaaf.

SparkQA · 2015-04-29T16:26:58Z

Test build #31286 has finished for PR 5776 at commit 29aaaaf.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-04-29T16:27:02Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-04-29T16:27:02Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31286/
Test PASSed.

scwf · 2015-04-30T00:00:59Z

/cc @marmbrus

marmbrus · 2015-04-30T00:08:59Z

@yhuai

yhuai · 2015-04-30T03:45:32Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

+   */
+  object CTESubstitution extends Rule[LogicalPlan] {
+    def apply(plan: LogicalPlan): LogicalPlan = {
+      val (realPlan, cteRelations) = plan match {


Do we still need to first get realPlan and creRelations? Can we just use the following?

def apply(plan: LogicalPlan): LogicalPlan = plan match { case With(child, relations) => substituteCTE(child, relations) case other => other }

yhuai · 2015-04-30T03:50:19Z

Thanks for working on it. I have left a few minor comments.

scwf · 2015-04-30T04:00:00Z

@yhuai ,thanks, updated!

AmplabJenkins · 2015-04-30T04:02:09Z

Merged build triggered.

AmplabJenkins · 2015-04-30T04:02:15Z

Merged build started.

SparkQA · 2015-04-30T04:03:43Z

Test build #31379 has started for PR 5776 at commit 0c54798.

yhuai · 2015-04-30T04:06:08Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

+            withAlias.getOrElse(relation)
+          }
+          substituted.getOrElse(u)
+          i.copy(table = relation)


i.copy(table = substituted.getOrElse(u))

oh, my bad, thanks

SparkQA · 2015-04-30T04:07:11Z

Test build #31379 has finished for PR 5776 at commit 0c54798.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

SparkQA · 2015-05-07T06:09:15Z

Test build #774 has started for PR 5776 at commit 553005a.

SparkQA · 2015-05-07T07:59:21Z

Test build #774 has finished for PR 5776 at commit 553005a.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

scwf · 2015-05-07T09:31:39Z

Retest this please

AmplabJenkins · 2015-05-07T09:32:11Z

Merged build triggered.

AmplabJenkins · 2015-05-07T09:32:18Z

Merged build started.

SparkQA · 2015-05-07T09:32:32Z

Test build #32101 has started for PR 5776 at commit 553005a.

SparkQA · 2015-05-07T11:30:27Z

Test build #32101 has finished for PR 5776 at commit 553005a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-05-07T11:30:31Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-05-07T11:30:32Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32101/
Test PASSed.

yhuai · 2015-05-07T15:37:25Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

+
+    def substituteCTE(plan: LogicalPlan, cteRelations: Map[String, LogicalPlan]): LogicalPlan = {
+      plan transform {
+        case i @ InsertIntoTable(u: UnresolvedRelation, _, _, _, _) =>


I feel this rule does nothing.... We are inserting to a table and this table is actually a CTE?

I feel we should get rid of this rule. I do not think this one actually works.

yes, i will delete this case

yhuai · 2015-05-07T19:02:52Z

@scwf Can you also move the the rule for handling window clause (https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala#L668-680) to this batch?

AmplabJenkins · 2015-05-08T03:07:11Z

Merged build triggered.

AmplabJenkins · 2015-05-08T03:07:20Z

Merged build started.

SparkQA · 2015-05-08T03:09:04Z

Test build #32183 has started for PR 5776 at commit d4b962f.

SparkQA · 2015-05-08T05:06:18Z

Test build #32183 has finished for PR 5776 at commit d4b962f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-05-08T05:06:22Z

Merged build finished. Test PASSed.

AmplabJenkins · 2015-05-08T05:06:22Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/32183/
Test PASSed.

scwf · 2015-05-08T05:54:40Z

Updated done /cc @yhuai

yhuai · 2015-05-08T05:55:03Z

Thanks! Merging to master and branch 1.4.

Added a new batch named `Substitution` before `Resolution` batch. The motivation for this is there are kind of cases we want to do some substitution on the parsed logical plan before resolve it. Consider this two cases: 1 CTE, for cte we first build a row logical plan ``` 'With Map(q1 -> 'Subquery q1 'Project ['key] 'UnresolvedRelation [src], None) 'Project [*] 'Filter ('key = 5) 'UnresolvedRelation [q1], None ``` In `With` logicalplan here is a map stored the (`q1-> subquery`), we want first take off the with command and substitute the `q1` of `UnresolvedRelation` by the `subquery` 2 Another example is Window function, in window function user may define some windows, we also need substitute the window name of child by the concrete window. this should also done in the Substitution batch. Author: wangfei <wangfei1@huawei.com> Closes #5776 from scwf/addbatch and squashes the following commits: d4b962f [wangfei] added WindowsSubstitution 70f6932 [wangfei] Merge branch 'master' of https://github.com/apache/spark into addbatch ecaeafb [wangfei] address yhuai's comments 553005a [wangfei] fix test case 0c54798 [wangfei] address comments 29aaaaf [wangfei] fix compile 1c9a092 [wangfei] added Substitution bastch (cherry picked from commit f496bf3) Signed-off-by: Yin Huai <yhuai@databricks.com>

Added a new batch named `Substitution` before `Resolution` batch. The motivation for this is there are kind of cases we want to do some substitution on the parsed logical plan before resolve it. Consider this two cases: 1 CTE, for cte we first build a row logical plan ``` 'With Map(q1 -> 'Subquery q1 'Project ['key] 'UnresolvedRelation [src], None) 'Project [*] 'Filter ('key = 5) 'UnresolvedRelation [q1], None ``` In `With` logicalplan here is a map stored the (`q1-> subquery`), we want first take off the with command and substitute the `q1` of `UnresolvedRelation` by the `subquery` 2 Another example is Window function, in window function user may define some windows, we also need substitute the window name of child by the concrete window. this should also done in the Substitution batch. Author: wangfei <wangfei1@huawei.com> Closes apache#5776 from scwf/addbatch and squashes the following commits: d4b962f [wangfei] added WindowsSubstitution 70f6932 [wangfei] Merge branch 'master' of https://github.com/apache/spark into addbatch ecaeafb [wangfei] address yhuai's comments 553005a [wangfei] fix test case 0c54798 [wangfei] address comments 29aaaaf [wangfei] fix compile 1c9a092 [wangfei] added Substitution bastch

scwf added 2 commits April 29, 2015 17:33

added Substitution bastch

1c9a092

fix compile

29aaaaf

scwf force-pushed the addbatch branch from 06257ca to 29aaaaf Compare April 29, 2015 10:21

yhuai reviewed Apr 30, 2015
View reviewed changes

address comments

0c54798

yhuai reviewed Apr 30, 2015
View reviewed changes

yhuai reviewed May 7, 2015
View reviewed changes

scwf added 3 commits May 8, 2015 10:35

address yhuai's comments

ecaeafb

Merge branch 'master' of https://github.com/apache/spark into addbatch

70f6932

added WindowsSubstitution

d4b962f

asfgit closed this in f496bf3 May 8, 2015

scwf deleted the addbatch branch May 8, 2015 06:05

[SPARK-7232] [SQL] Add a Substitution batch for spark sql analyzer #5776

[SPARK-7232] [SQL] Add a Substitution batch for spark sql analyzer #5776

Conversation

scwf commented Apr 29, 2015

AmplabJenkins commented Apr 29, 2015

AmplabJenkins commented Apr 29, 2015

SparkQA commented Apr 29, 2015

SparkQA commented Apr 29, 2015

AmplabJenkins commented Apr 29, 2015

AmplabJenkins commented Apr 29, 2015

scwf commented Apr 29, 2015

AmplabJenkins commented Apr 29, 2015

AmplabJenkins commented Apr 29, 2015

SparkQA commented Apr 29, 2015

SparkQA commented Apr 29, 2015

AmplabJenkins commented Apr 29, 2015

AmplabJenkins commented Apr 29, 2015

scwf commented Apr 30, 2015

marmbrus commented Apr 30, 2015

yhuai Apr 30, 2015

Choose a reason for hiding this comment

yhuai commented Apr 30, 2015

scwf commented Apr 30, 2015

AmplabJenkins commented Apr 30, 2015

AmplabJenkins commented Apr 30, 2015

SparkQA commented Apr 30, 2015

yhuai Apr 30, 2015

Choose a reason for hiding this comment

scwf Apr 30, 2015

Choose a reason for hiding this comment

SparkQA commented Apr 30, 2015

SparkQA commented May 7, 2015

SparkQA commented May 7, 2015

scwf commented May 7, 2015

AmplabJenkins commented May 7, 2015

AmplabJenkins commented May 7, 2015

SparkQA commented May 7, 2015

SparkQA commented May 7, 2015

AmplabJenkins commented May 7, 2015

AmplabJenkins commented May 7, 2015

yhuai May 7, 2015

Choose a reason for hiding this comment

yhuai May 7, 2015

Choose a reason for hiding this comment

scwf May 8, 2015

Choose a reason for hiding this comment

yhuai commented May 7, 2015

AmplabJenkins commented May 8, 2015

AmplabJenkins commented May 8, 2015

SparkQA commented May 8, 2015

SparkQA commented May 8, 2015

AmplabJenkins commented May 8, 2015

AmplabJenkins commented May 8, 2015

scwf commented May 8, 2015

yhuai commented May 8, 2015