Skip to content

[SPARK-17180] [SQL] Fix View Resolution Order in ALTER VIEW AS SELECT #14746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gatorsmile wants to merge 8 commits into from
Closed

[SPARK-17180] [SQL] Fix View Resolution Order in ALTER VIEW AS SELECT #14746

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
db45b4c
fix.
gatorsmile Aug 22, 2016
c5add2c
address comments.
gatorsmile Aug 25, 2016
51dbf72
address comments
gatorsmile Aug 26, 2016
d84db62
address comments.
gatorsmile Aug 27, 2016
47fdf1f
add ViewType
gatorsmile Aug 28, 2016
5606344
remove isAlterViewAsSelect
gatorsmile Aug 28, 2016
5c60b4c
Merge remote-tracking branch 'upstream/master' into alterViewFix
gatorsmile Aug 29, 2016
baa8a1f
address comments
gatorsmile Aug 29, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,13 @@ case class CreateViewCommand(
}
val sessionState = sparkSession.sessionState

if (isTemporary) {
// 1) CREATE VIEW: create a temp view when users explicitly specify the keyword TEMPORARY;
// otherwise, create a permanent view no matter whether the temporary view
// with the same name exists or not.
// 2) ALTER VIEW: alter the temporary view if the temp view exists; otherwise, try to alter
Copy link
Contributor

@cloud-fan cloud-fan Aug 25, 2016

Choose a reason for hiding this comment

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

question: how can you tell whether it's CREATE VIEW or ALTER VIEW?

Copy link
Member Author

@gatorsmile gatorsmile Aug 26, 2016

Choose a reason for hiding this comment

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

Yeah! The only way is to pass a flag.

Copy link
Contributor

@cloud-fan cloud-fan Aug 26, 2016

Choose a reason for hiding this comment

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

it already has 3 flags: allowExisting, replace, isTemporary. We should rethink about it and decide what flags we really need for this command.

Copy link
Member Author

@gatorsmile gatorsmile Aug 26, 2016

Choose a reason for hiding this comment

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

True, let me think about it. Thanks!

Copy link
Member Author

@gatorsmile gatorsmile Aug 26, 2016

Choose a reason for hiding this comment

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

      CREATE (OR REPLACE)? TEMPORARY? VIEW (IF NOT EXISTS)? tableIdentifier
        identifierCommentList? (COMMENT STRING)?
        (PARTITIONED ON identifierList)?
        (TBLPROPERTIES tablePropertyList)? AS query

Each flag corresponds to a keyword in the CREATE VIEW DDL command.

  • OR REPLACE -> replace
  • TEMPORARY -> isTemporary
  • IF NOT EXISTS -> allowExisting

If we want to use the same command CreateViewCommand to process both CREATE VIEW and ALTER VIEW, it sounds reasonable to add a new flag. So far, I have not found a way to combine them.

Copy link
Contributor

@cloud-fan cloud-fan Aug 27, 2016

Choose a reason for hiding this comment

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

like CreateMode, can we use SaveMode for replace and allowExisting?

Copy link
Member Author

@gatorsmile gatorsmile Aug 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

uh, I got it. : ) Copied what @liancheng wrote one month ago and did the update in the last case.

allowExisting replace SaveMode
true false Ignore
false false ErrorIfExists
false true Overwrite
true true AnalysisException

Let me try it. Thanks!

// the permanent view. Here, it follows the same resolution like DROP VIEW,
// since users are unable to specify the keyword TEMPORARY.
if (isTemporary || (replace && sessionState.catalog.isTemporaryTable(name))) {
createTemporaryView(sparkSession, analyzedPlan)
} else {
// Adds default database for permanent table if it doesn't exist, so that tableExists()
Expand Down
61 changes: 60 additions & 1 deletion sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
package org.apache.spark.sql.hive.execution

import org.apache.spark.sql.{AnalysisException, QueryTest, Row, SaveMode}
import org.apache.spark.sql.catalyst.TableIdentifier
import org.apache.spark.sql.catalyst.catalog.CatalogTableType
import org.apache.spark.sql.hive.test.TestHiveSingleton
import org.apache.spark.sql.test.SQLTestUtils

Expand Down Expand Up @@ -204,10 +206,67 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
}
}

test("should allow CREATE permanent VIEW when a TEMPORARY VIEW with same name exists") {
test("ALTER VIEW: alter a temporary view when a permanent VIEW with same name exists") {
alterTempView(isTempAlteredView = true)
}

test("ALTER VIEW: alter a persistent view when a temp VIEW with same name exists") {
alterTempView(isTempAlteredView = false)
}

private def alterTempView (isTempAlteredView: Boolean) = {
withView("testView", "default.testView") {
val catalog = spark.sessionState.catalog
val oldViewQuery = "SELECT id FROM jt"
val newViewQuery = "SELECT id, id1 FROM jt"
sql(s"CREATE VIEW default.testView AS $oldViewQuery")
sql(s"CREATE TEMPORARY VIEW testView AS $oldViewQuery")
if (isTempAlteredView) {
// When the database is not specified, we will first try to alter the temporary view
sql(s"ALTER VIEW testView AS $newViewQuery")
} else {
// When the database is specified, we will try to alter the permanent view, no matter
// whether the temporary view with the same name exists or not.
sql(s"ALTER VIEW default.testView AS $newViewQuery")
}

val persistentView = catalog.getTableMetadata(
TableIdentifier(table = "testView", database = Some("default")))
assert(persistentView.tableType == CatalogTableType.VIEW)
val tempView = catalog.getTableMetadata(TableIdentifier("testView"))
assert(tempView.tableType == CatalogTableType.VIEW)
assert(tempView.viewOriginalText.isEmpty)

if (isTempAlteredView) {
// View Text of the persistent view default.testView is not changed
assert(persistentView.viewOriginalText == Option(oldViewQuery))
// temp view testView is changed
checkAnswer(
sql(newViewQuery),
sql("select * from testView"))
} else {
// View Text of the persistent view default.testView is changed
assert(persistentView.viewOriginalText == Option(newViewQuery))
// temp view testView is not changed
checkAnswer(
sql(oldViewQuery),
sql("select * from testView"))
}
}
}

test("CREATE VIEW: should allow CREATE permanent VIEW when a temp VIEW with same name exists") {
withView("testView", "default.testView") {
sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt")
sql("CREATE VIEW testView AS SELECT id FROM jt")

// Both temporary and permanent view have been successfully created.
val catalog = spark.sessionState.catalog
val persistentView = catalog.getTableMetadata(
TableIdentifier(table = "testView", database = Some("default")))
assert(persistentView.tableType == CatalogTableType.VIEW)
val tempView = catalog.getTableMetadata(TableIdentifier("testView"))
assert(tempView.tableType == CatalogTableType.VIEW)
}
}

Expand Down