[SPARK-12573][SPARK-12574][SQL] Move SQL Parser from Hive to Catalyst #10583

hvanhovell · 2016-01-05T01:37:41Z

This PR moves a major part of the new SQL parser to Catalyst. This is a prelude to start using this parser for all of our SQL parsing. The following key changes have been made:

The ANTLR Parser & Supporting classes have been moved to the Catalyst project. They are now part of the org.apache.spark.sql.catalyst.parser package. These classes contained quite a bit of code that was originally from the Hive project, I have added aknowledgements whenever this applied. All Hive dependencies have been factored out. I have also taken this chance to clean-up the ASTNode class, and to improve the error handling.

The HiveQl object that provides the functionality to convert an AST into a LogicalPlan has been refactored into three different classes, one for every SQL sub-project:

CatalystQl: This implements Query and Expression parsing functionality.
SparkQl: This is a subclass of CatalystQL and provides SQL/Core only functionality such as Explain and Describe.
HiveQl: This is a subclass of SparkQl and this adds Hive-only functionality to the parser such as Analyze, Drop, Views, CTAS & Transforms. This class still depends on Hive.

cc @rxin

Factor out Hive dependencies - 2. Factor out hard coded UDFT's; let the Hive function registry deal resolve generators. Split Ql into Catalyst/Spark/Hive part; move parser to catalyst Style.

marmbrus · 2016-01-05T01:45:36Z

sql/catalyst/src/main/java/org/apache/spark/sql/parser/ParseUtils.java

Should we put this in catalyst? Just since we tend to hide non-public APIs there.

Yeah (reynold just suggested the same thing), I'll add it to catalyst.parser.

SparkQA · 2016-01-05T05:56:45Z

Test build #48709 has finished for PR 10583 at commit fb3b4a4.

This patch fails from timeout after a configured wait of 250m.
This patch merges cleanly.
This patch adds no public classes.

hvanhovell · 2016-01-05T08:04:07Z

retest this please

SparkQA · 2016-01-05T09:55:57Z

Test build #2322 has finished for PR 10583 at commit fb3b4a4.

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

SparkQA · 2016-01-05T10:22:54Z

Test build #48747 has finished for PR 10583 at commit fb3b4a4.

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

SparkQA · 2016-01-05T13:56:01Z

Test build #48766 has finished for PR 10583 at commit e397370.

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

SparkQA · 2016-01-05T16:21:53Z

Test build #48771 has finished for PR 10583 at commit 43c29b7.

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

SparkQA · 2016-01-05T20:27:49Z

Test build #48780 has finished for PR 10583 at commit 157d178.

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

hvanhovell · 2016-01-05T20:42:17Z

retest this please

SparkQA · 2016-01-05T23:17:22Z

Test build #48789 has finished for PR 10583 at commit 157d178.

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

rxin · 2016-01-05T23:40:22Z

Can you update the pull request description? It still says WIP.

hvanhovell · 2016-01-05T23:45:55Z

Done.

rxin · 2016-01-06T01:35:34Z

sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/parser/ParseUtils.java

looks like the indentation is off here?

rxin · 2016-01-06T02:05:56Z

cc @cloud-fan can you take a look at this? Thanks.

cloud-fan · 2016-01-06T02:41:28Z

sql/core/src/main/scala/org/apache/spark/sql/SQLConf.scala

should this be a boolean conf?

I ported this directly form Hive. It has only two options:

none: no quoting. I'll map this to false.

column: quoting. I'll map this to true.

rxin · 2016-01-06T03:19:25Z

BTW given the size of the pull request, I think we can also merge it provided that it has no structural problems, and then review feedback in follow-up prs.

# Conflicts: # sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveQl.scala # sql/hive/src/test/scala/org/apache/spark/sql/hive/ErrorPositionSuite.scala

hvanhovell · 2016-01-06T12:13:16Z

sql/core/src/main/scala/org/apache/spark/sql/SQLConf.scala

Some of our out-dated Hive Compatibility Tests are using SQL11 reserved keywords as identifiers. We should first fix those before we can set this flag to true.

SparkQA · 2016-01-06T13:45:32Z

Test build #48853 has finished for PR 10583 at commit 3680d4c.

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

marmbrus · 2016-01-06T17:59:20Z

Regarding the questions about LATERAL VIEW. Ideally I think we will support one query language that has a super-set of the features that were previously present in HiveQL and the SparkSQLParser. Where this is not possible I'd probably defer to HiveQL or a SQL Standard.

rxin · 2016-01-06T19:16:38Z

Due to the size of the patch, I'm going to merge this in now. @hvanhovell can address more comments as follow-up prs.

hvanhovell added 3 commits January 5, 2016 01:42

Factor out Hive dependencies.

774de5d

Factor out Hive dependencies - 2. Factor out hard coded UDFT's; let the Hive function registry deal resolve generators. Split Ql into Catalyst/Spark/Hive part; move parser to catalyst Style.

Changsed in Rollup/Cube

01ffaf1

Updated deps. Make test work again

fb3b4a4

hvanhovell changed the title ~~[SPARK-12573][SPARK-12574][SQL] Move Core Parser from Hive to Catalyst [WIP]~~ [SPARK-12573][SPARK-12574][SQL] Move SQL Parser from Hive to Catalyst [WIP] Jan 5, 2016

marmbrus reviewed Jan 5, 2016
View reviewed changes

Move parsers to private packages.

0a44c50

Fix Generator tests

e397370

Remove single quote from prettyString for python tests.

43c29b7

hvanhovell added 2 commits January 5, 2016 19:28

Merge remote-tracking branch 'spark/master' into SPARK-12575

cf7cf3a

Revert change to catalyst conf

157d178

hvanhovell changed the title ~~[SPARK-12573][SPARK-12574][SQL] Move SQL Parser from Hive to Catalyst [WIP]~~ [SPARK-12573][SPARK-12574][SQL] Move SQL Parser from Hive to Catalyst Jan 5, 2016

rxin reviewed Jan 6, 2016
View reviewed changes

sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/parser/ParseUtils.java Outdated

Copy link

Contributor

rxin Jan 6, 2016

Choose a reason for hiding this comment

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

looks like the indentation is off here?

cloud-fan reviewed Jan 6, 2016
View reviewed changes

hvanhovell added 9 commits January 6, 2016 09:31

Revert UnresolveGenerator

5b0b165

Style

62d5f17

Reinstate use of 'relativeToAbsolutePath' + docs

884daa4

Make PARSER_SUPPORT_QUOTEDID as boolean config var

1b969b4

Correct attribution

6e30471

Fix type.

8c79ab9

Improve error handling.

cc6eb6c

Merge remote-tracking branch 'spark/master' into SPARK-12575

6895a55

# Conflicts: # sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveQl.scala # sql/hive/src/test/scala/org/apache/spark/sql/hive/ErrorPositionSuite.scala

style

3680d4c

hvanhovell reviewed Jan 6, 2016
View reviewed changes

hvanhovell mentioned this pull request Jan 6, 2016

[SPARK-12577][SQL] Better support of parentheses in partition by and order by clause of window function's over clause #10620

Closed

rxin mentioned this pull request Jan 6, 2016

[SPARK-12593][SQL] Converts resolved logical plan back to SQL #10541

Closed

3 tasks

asfgit closed this in ea489f1 Jan 6, 2016

[SPARK-12573][SPARK-12574][SQL] Move SQL Parser from Hive to Catalyst #10583

[SPARK-12573][SPARK-12574][SQL] Move SQL Parser from Hive to Catalyst #10583

Uh oh!

Conversation

hvanhovell commented Jan 5, 2016

Uh oh!

marmbrus Jan 5, 2016

Choose a reason for hiding this comment

Uh oh!

hvanhovell Jan 5, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 5, 2016

Uh oh!

hvanhovell commented Jan 5, 2016

Uh oh!

SparkQA commented Jan 5, 2016

Uh oh!

SparkQA commented Jan 5, 2016

Uh oh!

SparkQA commented Jan 5, 2016

Uh oh!

SparkQA commented Jan 5, 2016

Uh oh!

SparkQA commented Jan 5, 2016

Uh oh!

hvanhovell commented Jan 5, 2016

Uh oh!

SparkQA commented Jan 5, 2016

Uh oh!

rxin commented Jan 5, 2016

Uh oh!

hvanhovell commented Jan 5, 2016

Uh oh!

rxin Jan 6, 2016

Choose a reason for hiding this comment

Uh oh!

rxin commented Jan 6, 2016

Uh oh!

cloud-fan Jan 6, 2016

Choose a reason for hiding this comment

Uh oh!

hvanhovell Jan 6, 2016

Choose a reason for hiding this comment

Uh oh!

rxin commented Jan 6, 2016

Uh oh!

hvanhovell Jan 6, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jan 6, 2016

Uh oh!

marmbrus commented Jan 6, 2016

Uh oh!

rxin commented Jan 6, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants