[SPARK-12719][SQL] SQL generation support for Generate #11696

cloud-fan · 2016-03-14T13:32:22Z

What changes were proposed in this pull request?

This PR adds SQL generation support for Generate operator. It always converts Generate operator into LATERAL VIEW format as there are many limitations to put UDTF in project list.

This PR is based on #11658, please see the last commit to review the real changes.

Thanks @dilipbiswal for his initial work! Takes over #11596

How was this patch tested?

new tests in LogicalPlanToSQLSuite

cloud-fan · 2016-03-14T13:33:17Z

cc @liancheng @yhuai @gatorsmile @dilipbiswal

SparkQA · 2016-03-14T13:49:59Z

Test build #53071 has finished for PR 11696 at commit cfc74ef.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

yhuai · 2016-03-14T15:40:25Z

test this please

SparkQA · 2016-03-14T17:35:44Z

Test build #53075 has finished for PR 11696 at commit cfc74ef.

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

dilipbiswal · 2016-03-14T19:07:45Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/SQLBuilder.scala

@cloud-fan Just for my understanding, do we miss some case now that we want to improve upon ?

According to SQL standard, the operator order should be SELECT ... FROM ... WHERE ... GROUP BY ... HAVING ... ORDER BY ... LIMIT ..., we should re-order operators to make them in standard order in the future.

@cloud-fan Thank you !!

let's put your comment actually in the code so we know why we need to re-order the operators

dilipbiswal · 2016-03-14T19:08:19Z

@cloud-fan Thank you. LGTM

gatorsmile · 2016-03-14T20:04:14Z

LGTM. Thank you for your work! @cloud-fan

rxin · 2016-03-15T06:31:33Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/SQLBuilder.scala

maybe just inline this since it is used only once

rxin · 2016-03-15T06:41:23Z

@cloud-fan the logic looks good to me.

I left some comments -- try make this understandable (by adding more comments or renaming some functions) for people that haven't spent a lot of time understanding this code.

yhuai · 2016-03-16T19:07:01Z

OK. I have merged https://github.com/apache/spark/pull/11658/commits. Let me rebase this one to just keep the top commit.

yhuai · 2016-03-16T19:27:01Z

#11768 is the commit for generator. To avoid of destroying @cloud-fan's branch by any accident, I create that new PR.

SparkQA · 2016-03-17T07:19:07Z

Test build #53402 has finished for PR 11696 at commit 6fca665.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-03-17T08:47:07Z

Test build #53404 has finished for PR 11696 at commit dcbbcb8.

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

cloud-fan · 2016-03-17T12:22:18Z

I'm going to merge it to unblock following works, will address comments of @yhuai and @liancheng if you have any.

liancheng · 2016-03-17T12:50:24Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/SQLBuilder.scala

  }

+  private def generateToSQL(g: Generate): String = {
+    val columnAliases = g.generatorOutput.map(_.sql).mkString(",")


Nit: Please add a space after ,

liancheng · 2016-03-17T13:01:40Z

Sorry for the late review. LGTM except for two minor comments.

…while generate SQL ## What changes were proposed in this pull request? We only need to make sub-query names unique every time we generate a SQL string, but not all the time. This PR moves the `newSubqueryName` method to `class SQLBuilder` and remove `object SQLBuilder`. also addressed 2 minor comments in #11696 ## How was this patch tested? existing tests. Author: Wenchen Fan <[email protected]> Closes #11783 from cloud-fan/tmp.

PR #11696 introduced a complex pattern match that broke Scala 2.10 match unreachability check and caused build failure. This PR fixes this issue by expanding this pattern match into several simpler ones. Note that tuning or turning off `-Dscalac.patmat.analysisBudget` doesn't work for this case. Compilation against Scala 2.10 Author: tedyu <[email protected]> Closes #11798 from yy2016/master.

## What changes were proposed in this pull request? This PR adds SQL generation support for `Generate` operator. It always converts `Generate` operator into `LATERAL VIEW` format as there are many limitations to put UDTF in project list. This PR is based on apache#11658, please see the last commit to review the real changes. Thanks dilipbiswal for his initial work! Takes over apache#11596 ## How was this patch tested? new tests in `LogicalPlanToSQLSuite` Author: Wenchen Fan <[email protected]> Closes apache#11696 from cloud-fan/generate.

…while generate SQL ## What changes were proposed in this pull request? We only need to make sub-query names unique every time we generate a SQL string, but not all the time. This PR moves the `newSubqueryName` method to `class SQLBuilder` and remove `object SQLBuilder`. also addressed 2 minor comments in apache#11696 ## How was this patch tested? existing tests. Author: Wenchen Fan <[email protected]> Closes apache#11783 from cloud-fan/tmp.

PR apache#11696 introduced a complex pattern match that broke Scala 2.10 match unreachability check and caused build failure. This PR fixes this issue by expanding this pattern match into several simpler ones. Note that tuning or turning off `-Dscalac.patmat.analysisBudget` doesn't work for this case. Compilation against Scala 2.10 Author: tedyu <[email protected]> Closes apache#11798 from yy2016/master.

cloud-fan mentioned this pull request Mar 14, 2016

[SPARK-12719][SQL] [WIP] SQL generation support for generators, including UDTF #11596

Closed

dilipbiswal reviewed Mar 14, 2016
View reviewed changes

dilipbiswal mentioned this pull request Mar 14, 2016

[SPARK-12719][SQL] SQL generation support for generators, including UDTF #11563

Closed

rxin reviewed Mar 15, 2016
View reviewed changes

sql/hive/src/main/scala/org/apache/spark/sql/hive/SQLBuilder.scala Outdated

Copy link

Contributor

rxin Mar 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe just inline this since it is used only once

sql generation for Generate

6fca665

cloud-fan force-pushed the generate branch from cfc74ef to 6fca665 Compare March 17, 2016 07:12

fix style

dcbbcb8

asfgit closed this in 1974d1d Mar 17, 2016

liancheng reviewed Mar 17, 2016
View reviewed changes

cloud-fan mentioned this pull request Mar 17, 2016

[SPARK-13974][SQL] sub-query names do not need to be globally unique while generate SQL #11783

Closed

liancheng mentioned this pull request Mar 18, 2016

[SPARK-12719][HOTFIX] Fix compilation against Scala 2.10 #11798

Closed

[SPARK-12719][SQL] SQL generation support for Generate #11696

[SPARK-12719][SQL] SQL generation support for Generate #11696

Uh oh!

Conversation

cloud-fan commented Mar 14, 2016

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

cloud-fan commented Mar 14, 2016

Uh oh!

SparkQA commented Mar 14, 2016

Uh oh!

yhuai commented Mar 14, 2016

Uh oh!

SparkQA commented Mar 14, 2016

Uh oh!

dilipbiswal Mar 14, 2016

Choose a reason for hiding this comment

Uh oh!

cloud-fan Mar 15, 2016

Choose a reason for hiding this comment

Uh oh!

dilipbiswal Mar 15, 2016

Choose a reason for hiding this comment

Uh oh!

rxin Mar 15, 2016

Choose a reason for hiding this comment

Uh oh!

dilipbiswal commented Mar 14, 2016

Uh oh!

gatorsmile commented Mar 14, 2016

Uh oh!

rxin Mar 15, 2016

Choose a reason for hiding this comment

Uh oh!

rxin commented Mar 15, 2016

Uh oh!

yhuai commented Mar 16, 2016

Uh oh!

yhuai commented Mar 16, 2016

Uh oh!

SparkQA commented Mar 17, 2016

Uh oh!

SparkQA commented Mar 17, 2016

Uh oh!

cloud-fan commented Mar 17, 2016

Uh oh!

liancheng Mar 17, 2016

Choose a reason for hiding this comment

Uh oh!

liancheng commented Mar 17, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants