Skip to content

Commit

Permalink
[SPARK-29730][SQL] ALTER VIEW QUERY should look up catalog/table like…
Browse files Browse the repository at this point in the history
… v2 commands

Add AlterViewAsStatement and make ALTER VIEW ... QUERY go through the same catalog/table resolution framework of v2 commands.

It's important to make all the commands have the same table resolution behavior, to avoid confusing end-users. e.g.
```
USE my_catalog
DESC v // success and describe the view v from my_catalog
ALTER VIEW v SELECT 1 // report view not found as there is no view v in the session catalog
```

Yes. When running ALTER VIEW ... QUERY, Spark fails the command if the current catalog is set to a v2 catalog, or the view name specified a v2 catalog.

unit tests

Closes apache#26453 from huaxingao/spark-29730.

Authored-by: Huaxin Gao <huaxing@us.ibm.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
  • Loading branch information
huaxingao authored and dongjoon-hyun committed Nov 10, 2019
1 parent 2888009 commit 57b954e
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 16 deletions.
Expand Up @@ -178,7 +178,7 @@ statement
| CREATE (OR REPLACE)? GLOBAL? TEMPORARY VIEW
tableIdentifier ('(' colTypeList ')')? tableProvider
(OPTIONS tablePropertyList)? #createTempViewUsing
| ALTER VIEW tableIdentifier AS? query #alterViewQuery
| ALTER VIEW multipartIdentifier AS? query #alterViewQuery
| CREATE (OR REPLACE)? TEMPORARY? FUNCTION (IF NOT EXISTS)?
multipartIdentifier AS className=STRING
(USING resource (',' resource)*)? #createFunction
Expand Down
Expand Up @@ -3165,4 +3165,19 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
// TODO a partition spec is allowed to have optional values. This is currently violated.
Option(ctx.partitionSpec).map(visitNonOptionalPartitionSpec))
}

/**
* Alter the query of a view. This creates a [[AlterViewAsStatement]]
*
* For example:
* {{{
* ALTER VIEW multi_part_name AS SELECT ...;
* }}}
*/
override def visitAlterViewQuery(ctx: AlterViewQueryContext): LogicalPlan = withOrigin(ctx) {
AlterViewAsStatement(
visitMultipartIdentifier(ctx.multipartIdentifier),
originalText = source(ctx.query),
query = plan(ctx.query))
}
}
Expand Up @@ -238,6 +238,13 @@ case class AlterViewUnsetPropertiesStatement(
propertyKeys: Seq[String],
ifExists: Boolean) extends ParsedStatement

/**
* ALTER VIEW ... Query command, as parsed from SQL.
*/
case class AlterViewAsStatement(
viewName: Seq[String],
originalText: String,
query: LogicalPlan) extends ParsedStatement

/**
* A DROP TABLE statement, as parsed from SQL.
Expand Down
Expand Up @@ -1579,6 +1579,13 @@ class DDLParserSuite extends AnalysisTest {
comparePlans(parsed7, expected7)
}

test("alter view: AS Query") {
val parsed = parsePlan("ALTER VIEW a.b.c AS SELECT 1")
val expected = AlterViewAsStatement(
Seq("a", "b", "c"), "SELECT 1", parsePlan("SELECT 1"))
comparePlans(parsed, expected)
}

private case class TableSpec(
name: Seq[String],
schema: Option[StructType],
Expand Down
Expand Up @@ -421,6 +421,13 @@ class ResolveSessionCatalog(
serdeClassName,
serdeProperties,
partitionSpec)

case AlterViewAsStatement(tableName, originalText, query) =>
val v1TableName = parseV1Table(tableName, "ALTER VIEW QUERY")
AlterViewAsCommand(
v1TableName.asTableIdentifier,
originalText,
query)
}

private def parseV1Table(tableName: Seq[String], sql: String): Seq[String] = {
Expand Down
Expand Up @@ -858,21 +858,6 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder(conf) {
viewType = viewType)
}

/**
* Alter the query of a view. This creates a [[AlterViewAsCommand]] command.
*
* For example:
* {{{
* ALTER VIEW [db_name.]view_name AS SELECT ...;
* }}}
*/
override def visitAlterViewQuery(ctx: AlterViewQueryContext): LogicalPlan = withOrigin(ctx) {
AlterViewAsCommand(
name = visitTableIdentifier(ctx.tableIdentifier),
originalText = source(ctx.query),
query = plan(ctx.query))
}

/**
* Create a [[ScriptInputOutputSchema]].
*/
Expand Down
Expand Up @@ -1554,6 +1554,14 @@ class DataSourceV2SQLSuite
}
}

test("ALTER VIEW AS QUERY") {
val v = "testcat.ns1.ns2.v"
val e = intercept[AnalysisException] {
sql(s"ALTER VIEW $v AS SELECT 1")
}
assert(e.message.contains("ALTER VIEW QUERY is only supported with v1 tables"))
}

private def testV1Command(sqlCommand: String, sqlParams: String): Unit = {
val e = intercept[AnalysisException] {
sql(s"$sqlCommand $sqlParams")
Expand Down

0 comments on commit 57b954e

Please sign in to comment.