[SPARK-14182] [SQL] Parse DDL Command: Alter View #11987

gatorsmile · 2016-03-27T07:58:38Z

What changes were proposed in this pull request?

This PR is to provide native parsing support for DDL commands: Alter View. Since its AST trees are highly similar to Alter Table. Thus, both implementation are integrated into the same one.

Based on the Hive DDL document:
https://cwiki.apache.org/confluence/display/Hive/LanguageManual+DDL and https://cwiki.apache.org/confluence/display/Hive/PartitionedViews

1. ALTER VIEW RENAME

Syntax:

ALTER VIEW view_name RENAME TO new_view_name

to change the name of a view to a different name

2. ALTER VIEW SET TBLPROPERTIES

Syntax:

ALTER VIEW view_name SET TBLPROPERTIES ('comment' = new_comment);

to add metadata to a view

3. ALTER VIEW UNSET TBLPROPERTIES

Syntax:

ALTER VIEW view_name UNSET TBLPROPERTIES [IF EXISTS] ('comment', 'key')

to remove metadata from a view

4. ALTER VIEW ADD PARTITION

Syntax:

ALTER VIEW view_name ADD [IF NOT EXISTS] PARTITION spec1[, PARTITION spec2, ...]

to add the partitioning metadata for a view.
the syntax of partition spec in ALTER VIEW is identical to ALTER TABLE, EXCEPT that it is ILLEGAL to specify a LOCATION clause.

5. ALTER VIEW DROP PARTITION

Syntax:

ALTER VIEW view_name DROP [IF EXISTS] PARTITION spec1[, PARTITION spec2, ...]

to drop the related partition metadata for a view.

How was this patch tested?

Added the related test cases to DDLCommandSuite

# Conflicts: # sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala

SparkQA · 2016-03-30T17:26:57Z

Test build #54528 has finished for PR 11987 at commit f379636.

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

hvanhovell · 2016-03-30T19:15:58Z

sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/ng/SqlBase.g4

@@ -102,6 +102,15 @@ statement
        (PARTITIONED ON identifierList)?
        (TBLPROPERTIES tablePropertyList)? AS query                    #createView
    | ALTER VIEW tableIdentifier AS? query                             #alterViewQuery
+    | ALTER VIEW from=tableIdentifier AS? RENAME TO to=tableIdentifier #renameView


I don't think we need a dedicated label (i.e. #renameView) for this, we can just re-use #renameTable for this. That way you don't have to add any code to SparkSqlParser. The same goes for all these commands.

We can also integrate this with the ALTER TABLE ... equivalent rules (i.e. ALTER (TABLE|VIEW) .... But this is a bit messier, and I'll leave that to you.

Fine, I will combine them for renameView, setViewProperties, unsetViewProperties.

I still want to keep addViewPartition and dropViewPartitions. These two have different syntax. Different from Alter Table, addViewPartition is unable to accept the location clauses; dropViewPartitions is unable to accept PURGE.

I am wondering if that makes sense? Thanks!

It does. You can keep the grammar for addViewPartition and dropViewPartitions, but reuse the labels. The parser will then catch unallowed syntax, and we will not duplicate code.

Great! Will do the change. Thanks!

viirya · 2016-03-31T04:57:42Z

sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/ng/SqlBase.g4

@@ -74,12 +75,16 @@ statement
        SET SKEWED LOCATION skewedLocationList                         #setTableSkewLocations
    | ALTER TABLE tableIdentifier ADD (IF NOT EXISTS)?
        partitionSpecLocation+                                         #addTablePartition
+    | ALTER VIEW tableIdentifier ADD (IF NOT EXISTS)?
+        partitionSpec+                                                 #addViewPartition


re-use label?

@hvanhovell Still trying to combine addViewPartition and addTablePartition. Will keep you posted. Thanks!

@viirya Yeah, I am doing it now.

SparkQA · 2016-03-31T06:14:13Z

Test build #54594 has finished for PR 11987 at commit dd34529.

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

SparkQA · 2016-03-31T06:16:48Z

Test build #54592 has finished for PR 11987 at commit 38ea348.

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

SparkQA · 2016-03-31T07:18:33Z

Test build #54598 has finished for PR 11987 at commit 48aec92.

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

gatorsmile · 2016-03-31T07:25:29Z

@andrewor14 @rxin Based on your comments, the latest commits removed all the changes on the ANTLR3 codes, which will be removed soon. Now, the only changes in this PR are against ANTLR4.

@hvanhovell @viirya Please check if the changes on Parser is clean and clear.

Thank you! : )

viirya · 2016-03-31T08:24:06Z

sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala

+ * Add Partition in ALTER TABLE/VIEW: add the table/view partitions.
+ * 'partitionSpecsAndLocs': the syntax of ALTER VIEW is identical to ALTER TABLE,
+ * EXCEPT that it is ILLEGAL to specify a LOCATION clause.
+ * An error message will be issued if the partition exists, unless 'ifNotExists' is false.


unless 'ifNotExists' is false. -> unless 'ifNotExists' is true ?

Thanks, fixed.

hvanhovell · 2016-03-31T14:27:42Z

sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala

-        val spec = visitNonOptionalPartitionSpec(splCtx.partitionSpec)
-        val location = Option(splCtx.locationSpec).map(visitLocationSpec)
-        spec -> location
+    val specsAndLocs = if (ctx.partitionSpecLocation.isEmpty) {


Minor: I slightly prefer checking if partitionSpec is non empty. This is effectively the same, since they are mutally exclusive.

Sure, I can make a quick change. Thanks!

hvanhovell · 2016-03-31T14:29:27Z

One minor comment. LGTM pending jenkins.

viirya · 2016-03-31T14:44:21Z

LGTM

SparkQA · 2016-03-31T15:21:05Z

Test build #54628 has finished for PR 11987 at commit cdb8cc0.

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

SparkQA · 2016-03-31T16:25:57Z

Test build #54634 has finished for PR 11987 at commit 315de90.

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

# Conflicts: # sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4

SparkQA · 2016-03-31T18:16:08Z

Test build #54644 has finished for PR 11987 at commit dd1aa23.

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

gatorsmile · 2016-03-31T18:39:00Z

@andrewor14 @yhuai Could you also check if the latest version is fine? Thanks!

andrewor14 · 2016-03-31T18:59:35Z

LGTM merging into master, thanks @gatorsmile and reviewers!

gatorsmile and others added 30 commits November 13, 2015 14:50

Merge remote-tracking branch 'upstream/master'

01e4cdf

Merge remote-tracking branch 'upstream/master'

6835704

Merge remote-tracking branch 'upstream/master'

9180687

SPARK-11633

b38a21e

Merge remote-tracking branch 'upstream/master' into joinMakeCopy

d2b84af

Merge remote-tracking branch 'upstream/master'

fda8025

Merge branch 'master' of https://github.com/gatorsmile/spark

ac0dccd

Merge remote-tracking branch 'upstream/master'

6e0018b

converge

0546772

converge

b37a64f

Merge remote-tracking branch 'upstream/master'

c2a872c

Merge remote-tracking branch 'upstream/master'

ab6dbd7

Merge remote-tracking branch 'upstream/master'

4276356

Merge remote-tracking branch 'upstream/master'

2dab708

Merge remote-tracking branch 'upstream/master'

0458770

Merge remote-tracking branch 'upstream/master'

1debdfa

Merge remote-tracking branch 'upstream/master'

763706d

Merge remote-tracking branch 'upstream/master'

4de6ec1

Merge remote-tracking branch 'upstream/master'

9422a4f

Merge remote-tracking branch 'upstream/master'

52bdf48

Merge remote-tracking branch 'upstream/master'

1e95df3

Merge remote-tracking branch 'upstream/master'

fab24cf

Merge remote-tracking branch 'upstream/master'

8b2e33b

Merge remote-tracking branch 'upstream/master'

2ee1876

Merge remote-tracking branch 'upstream/master'

b9f0090

Merge remote-tracking branch 'upstream/master'

ade6f7e

Merge remote-tracking branch 'upstream/master'

9fd63d2

Merge remote-tracking branch 'upstream/master'

5199d49

Merge remote-tracking branch 'upstream/master'

404214c

Merge remote-tracking branch 'upstream/master'

c001dd9

gatorsmile added 5 commits March 29, 2016 14:32

Merge remote-tracking branch 'upstream/master'

07afea5

Merge remote-tracking branch 'upstream/master'

8bf2007

Merge branch 'parseAlterView' into parseAlterViewNew

de66cff

# Conflicts: # sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala

address comments.

9136b58

add test cases.

f379636

hvanhovell reviewed Mar 30, 2016
View reviewed changes

gatorsmile added 2 commits March 30, 2016 21:46

address comments.

38ea348

address comments.

dd34529

viirya reviewed Mar 31, 2016
View reviewed changes

address comments.

48aec92

viirya reviewed Mar 31, 2016
View reviewed changes

update the comments.

cdb8cc0

hvanhovell reviewed Mar 31, 2016
View reviewed changes

address comments.

315de90

gatorsmile added 2 commits March 31, 2016 09:32

Merge remote-tracking branch 'upstream/master'

87a165b

Merge branch 'parseAlterViewNew' into parseAlterViewNewNew

dd1aa23

# Conflicts: # sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4

asfgit closed this in 446c45b Mar 31, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-14182] [SQL] Parse DDL Command: Alter View #11987

[SPARK-14182] [SQL] Parse DDL Command: Alter View #11987

gatorsmile commented Mar 27, 2016

SparkQA commented Mar 30, 2016

hvanhovell Mar 30, 2016

gatorsmile Mar 30, 2016

hvanhovell Mar 30, 2016

gatorsmile Mar 30, 2016

viirya Mar 31, 2016

gatorsmile Mar 31, 2016

gatorsmile Mar 31, 2016

SparkQA commented Mar 31, 2016

SparkQA commented Mar 31, 2016

SparkQA commented Mar 31, 2016

gatorsmile commented Mar 31, 2016

viirya Mar 31, 2016

gatorsmile Mar 31, 2016

hvanhovell Mar 31, 2016

gatorsmile Mar 31, 2016

hvanhovell commented Mar 31, 2016

viirya commented Mar 31, 2016

SparkQA commented Mar 31, 2016

SparkQA commented Mar 31, 2016

SparkQA commented Mar 31, 2016

gatorsmile commented Mar 31, 2016

andrewor14 commented Mar 31, 2016

[SPARK-14182] [SQL] Parse DDL Command: Alter View #11987

[SPARK-14182] [SQL] Parse DDL Command: Alter View #11987

Conversation

gatorsmile commented Mar 27, 2016

What changes were proposed in this pull request?

1. ALTER VIEW RENAME

2. ALTER VIEW SET TBLPROPERTIES

3. ALTER VIEW UNSET TBLPROPERTIES

4. ALTER VIEW ADD PARTITION

5. ALTER VIEW DROP PARTITION

How was this patch tested?

SparkQA commented Mar 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Mar 31, 2016

SparkQA commented Mar 31, 2016

SparkQA commented Mar 31, 2016

gatorsmile commented Mar 31, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hvanhovell commented Mar 31, 2016

viirya commented Mar 31, 2016

SparkQA commented Mar 31, 2016

SparkQA commented Mar 31, 2016

SparkQA commented Mar 31, 2016

gatorsmile commented Mar 31, 2016

andrewor14 commented Mar 31, 2016