From ce139a6af184242e3c9ac7c368e9e864b6877dce Mon Sep 17 00:00:00 2001 From: Sean Zhong Date: Sat, 30 Apr 2016 17:08:18 +0800 Subject: [PATCH 1/5] * [SPARK-6339][SQL] Supports create CREATE TEMPORARY VIEW tableIdentifier AS query This PR support new SQL syntax CREATE TEMPORARY VIEW. Unit tests. Author: Sean Zhong --- .../spark/sql/catalyst/parser/SqlBase.g4 | 2 +- .../spark/sql/execution/SparkSqlParser.scala | 13 +- .../spark/sql/execution/command/views.scala | 88 ++++++++--- .../spark/sql/hive/HiveDDLCommandSuite.scala | 2 +- .../sql/hive/execution/SQLViewSuite.scala | 137 ++++++++++++++++-- 5 files changed, 207 insertions(+), 35 deletions(-) diff --git a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 index cc4e5c853e67..3df44f91e75d 100644 --- a/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 +++ b/sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 @@ -94,7 +94,7 @@ statement | DROP TABLE (IF EXISTS)? tableIdentifier PURGE? (FOR METADATA? REPLICATION '(' STRING ')')? #dropTable | DROP VIEW (IF EXISTS)? tableIdentifier #dropTable - | CREATE (OR REPLACE)? VIEW (IF NOT EXISTS)? tableIdentifier + | CREATE (OR REPLACE)? TEMPORARY? VIEW (IF NOT EXISTS)? tableIdentifier identifierCommentList? (COMMENT STRING)? (PARTITIONED ON identifierList)? (TBLPROPERTIES tablePropertyList)? AS query #createView diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala index dfc56a7d98ba..6e6fc77025c9 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala @@ -1032,7 +1032,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { * * For example: * {{{ - * CREATE VIEW [IF NOT EXISTS] [db_name.]view_name + * CREATE [TEMPORARY] VIEW [IF NOT EXISTS] [db_name.]view_name * [(column_name [COMMENT column_comment], ...) ] * [COMMENT view_comment] * [TBLPROPERTIES (property_name = property_value, ...)] @@ -1055,7 +1055,8 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { ctx.query, Option(ctx.tablePropertyList).map(visitTablePropertyList).getOrElse(Map.empty), ctx.EXISTS != null, - ctx.REPLACE != null + ctx.REPLACE != null, + ctx.TEMPORARY != null ) } } @@ -1072,7 +1073,8 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { ctx.query, Map.empty, allowExist = false, - replace = true) + replace = true, + isTemporary = false) } /** @@ -1086,7 +1088,8 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { query: QueryContext, properties: Map[String, String], allowExist: Boolean, - replace: Boolean): LogicalPlan = { + replace: Boolean, + isTemporary: Boolean): LogicalPlan = { val sql = Option(source(query)) val tableDesc = CatalogTable( identifier = visitTableIdentifier(name), @@ -1097,7 +1100,7 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder { viewOriginalText = sql, viewText = sql, comment = comment) - CreateViewCommand(tableDesc, plan(query), allowExist, replace, command(ctx)) + CreateViewCommand(tableDesc, plan(query), allowExist, replace, isTemporary, command(ctx)) } /** diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala index 1641780db8bc..aecc6a2fee1d 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala @@ -20,7 +20,7 @@ package org.apache.spark.sql.execution.command import scala.util.control.NonFatal import org.apache.spark.sql.{AnalysisException, Row, SparkSession} -import org.apache.spark.sql.catalyst.SQLBuilder +import org.apache.spark.sql.catalyst.{SQLBuilder, TableIdentifier} import org.apache.spark.sql.catalyst.catalog.{CatalogColumn, CatalogTable, CatalogTableType} import org.apache.spark.sql.catalyst.expressions.{Alias, Attribute} import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Project} @@ -37,6 +37,10 @@ import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Project} * already exists, throws analysis exception. * @param replace if true, and if the view already exists, updates it; if false, and if the view * already exists, throws analysis exception. + * @param isTemporary if true, the view is created as a temporary view. Temporary views are dropped + * at the end of current Spark session. Existing permanent relations with the same + * name are not visible to the current session while the temporary view exists, + * unless they are specified with full qualified table name with database prefix. * @param sql the original sql */ case class CreateViewCommand( @@ -44,6 +48,7 @@ case class CreateViewCommand( child: LogicalPlan, allowExisting: Boolean, replace: Boolean, + isTemporary: Boolean, sql: String) extends RunnableCommand { @@ -55,13 +60,18 @@ case class CreateViewCommand( require(tableDesc.tableType == CatalogTableType.VIEW) require(tableDesc.viewText.isDefined) - private val tableIdentifier = tableDesc.identifier - if (allowExisting && replace) { throw new AnalysisException( "It is not allowed to define a view with both IF NOT EXISTS and OR REPLACE.") } + // Temporary view names should NOT contain database prefix like "database.table" + if (isTemporary && tableDesc.identifier.database.isDefined) { + val database = tableDesc.identifier.database.get + throw new AnalysisException( + s"It is not allowed to add database prefix ${database} for the TEMPORARY view name.") + } + override def run(sparkSession: SparkSession): Seq[Row] = { // If the plan cannot be analyzed, throw an exception and don't proceed. val qe = sparkSession.executePlan(child) @@ -71,27 +81,69 @@ case class CreateViewCommand( require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length) val sessionState = sparkSession.sessionState - if (sessionState.catalog.tableExists(tableIdentifier)) { + if (isTemporary) { + createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan) + } else { + // Adds default database for permanent table if it doesn't exist, so that tableExists() + // only check permanent tables. + val database = tableDesc.identifier.database.getOrElse( + sessionState.catalog.getCurrentDatabase) + val tableIdentifier = tableDesc.identifier.copy(database = Option(database)) + + if (sessionState.catalog.tableExists(tableIdentifier)) { + if (allowExisting) { + // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view + // already exists. + } else if (replace) { + // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...` + sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan)) + } else { + // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already + // exists. + throw new AnalysisException(s"View $tableIdentifier already exists. " + + "If you want to update the view definition, please use ALTER VIEW AS or " + + "CREATE OR REPLACE VIEW AS") + } + } else { + // Create the view if it doesn't exist. + sessionState.catalog.createTable( + prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false) + } + } + Seq.empty[Row] + } + + private def createTemporaryView(table: TableIdentifier, sparkSession: SparkSession, + analyzedPlan: LogicalPlan): Unit = { + + val sessionState = sparkSession.sessionState + val catalog = sessionState.catalog + + // Projects column names to alias names + val logicalPlan = { + if (tableDesc.schema.isEmpty) { + analyzedPlan + } else { + val projectList = analyzedPlan.output.zip(tableDesc.schema).map { + case (attr, col) => Alias(attr, col.name)() + } + sparkSession.executePlan(Project(projectList, analyzedPlan)).analyzed + } + } + + if (catalog.tableExists(table) && catalog.isTemporaryTable(table)) { if (allowExisting) { - // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view - // already exists. + // Does nothing when the target view already exists. } else if (replace) { - // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...` - sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan)) + // Replaces the temp view if it exists + catalog.createTempTable(table.table, logicalPlan, replace) } else { - // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already - // exists. - throw new AnalysisException(s"View $tableIdentifier already exists. " + - "If you want to update the view definition, please use ALTER VIEW AS or " + - "CREATE OR REPLACE VIEW AS") + throw new AnalysisException(s"Temporary view $table already exists. ") } } else { - // Create the view if it doesn't exist. - sessionState.catalog.createTable( - prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false) + // Creates a temp view + catalog.createTempTable(table.table, logicalPlan, overrideIfExists = false) } - - Seq.empty[Row] } /** diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveDDLCommandSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveDDLCommandSuite.scala index 8dc3c6435327..de2a5c0140eb 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveDDLCommandSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveDDLCommandSuite.scala @@ -39,7 +39,7 @@ class HiveDDLCommandSuite extends PlanTest { parser.parsePlan(sql).collect { case CreateTable(desc, allowExisting) => (desc, allowExisting) case CreateTableAsSelectLogicalPlan(desc, _, allowExisting) => (desc, allowExisting) - case CreateViewCommand(desc, _, allowExisting, _, _) => (desc, allowExisting) + case CreateViewCommand(desc, _, allowExisting, _, _, _) => (desc, allowExisting) }.head } diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala index 0d88b3b87f50..91cf1c7870a5 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala @@ -37,11 +37,21 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { sqlContext.sql(s"DROP TABLE IF EXISTS jt") } - test("nested views") { - withView("jtv1", "jtv2") { + test("nested views (interleaved with temporary views)") { + withView("jtv1", "jtv2", "jtv3", "temp_jtv1", "temp_jtv2", "temp_jtv3") { sql("CREATE VIEW jtv1 AS SELECT * FROM jt WHERE id > 3").collect() sql("CREATE VIEW jtv2 AS SELECT * FROM jtv1 WHERE id < 6").collect() checkAnswer(sql("select count(*) FROM jtv2"), Row(2)) + + // Checks temporary views + sql("CREATE TEMPORARY VIEW temp_jtv1 AS SELECT * FROM jt WHERE id > 3").collect() + sql("CREATE TEMPORARY VIEW temp_jtv2 AS SELECT * FROM temp_jtv1 WHERE id < 6").collect() + checkAnswer(sql("select count(*) FROM temp_jtv2"), Row(2)) + + // Checks interleaved temporary view and normal view + sql("CREATE TEMPORARY VIEW temp_jtv3 AS SELECT * FROM jt WHERE id > 3").collect() + sql("CREATE VIEW jtv3 AS SELECT * FROM temp_jtv3 WHERE id < 6").collect() + checkAnswer(sql("select count(*) FROM jtv3"), Row(2)) } } @@ -57,6 +67,27 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { } } + test("error handling: fail if the temp view name contains the database prefix") { + // Full qualified table name is not allowed + val e = intercept[AnalysisException] { + sql("CREATE OR REPLACE TEMPORARY VIEW default.myabcdview AS SELECT * FROM jt").collect() + } + assert(e.message.contains("It is not allowed to add database prefix")) + } + + test("error handling: fail if the temp view sql itself is invalid") { + // A table that does not exist for temporary view + intercept[AnalysisException] { + sql("CREATE OR REPLACE TEMPORARY VIEW myabcdview AS SELECT * FROM table_not_exist1345") + .collect() + } + + // A column that does not exist, for temporary view + intercept[AnalysisException] { + sql("CREATE OR REPLACE TEMPORARY VIEW myabcdview AS SELECT random1234 FROM jt").collect() + } + } + test("correctly parse CREATE VIEW statement") { withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { sql( @@ -69,18 +100,105 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { } } + test("correctly parse CREATE TEMPORARY VIEW statement") { + withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { + withView("testView") { + sql( + """CREATE TEMPORARY VIEW IF NOT EXISTS + |testView (c1 COMMENT 'blabla', c2 COMMENT 'blabla') + |TBLPROPERTIES ('a' = 'b') + |AS SELECT * FROM jt""".stripMargin) + checkAnswer(sql("SELECT c1, c2 FROM testView ORDER BY c1"), (1 to 9).map(i => Row(i, i))) + } + } + } + + test("should NOT allow CREATE TEMPORARY VIEW when TEMPORARY VIEW with same name exists") { + withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { + withView("testView") { + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") + + val e = intercept[AnalysisException] { + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect() + } + + assert(e.message.contains("Temporary view") && e.message.contains("already exists")) + } + } + } + + test("should allow CREATE TEMPORARY VIEW when a permanent VIEW with same name exists") { + withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { + withView("testView", "default.testView") { + sql("CREATE VIEW testView AS SELECT id FROM jt") + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect() + } + } + } + + test("should allow CREATE permanent VIEW when a TEMPORARY VIEW with same name exists") { + withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { + withView("testView", "default.testView") { + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") + sql("CREATE VIEW testView AS SELECT id FROM jt").collect() + } + } + } + test("correctly handle CREATE VIEW IF NOT EXISTS") { withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { withTable("jt2") { - sql("CREATE VIEW testView AS SELECT id FROM jt") + withView("testView") { + sql("CREATE VIEW testView AS SELECT id FROM jt") - val df = (1 until 10).map(i => i -> i).toDF("i", "j") - df.write.format("json").saveAsTable("jt2") - sql("CREATE VIEW IF NOT EXISTS testView AS SELECT * FROM jt2") + val df = (1 until 10).map(i => i -> i).toDF("i", "j") + df.write.format("json").saveAsTable("jt2") + sql("CREATE VIEW IF NOT EXISTS testView AS SELECT * FROM jt2") - // make sure our view doesn't change. - checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i))) - sql("DROP VIEW testView") + // make sure our view doesn't change. + checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i))) + } + } + } + } + + test("correctly handle CREATE TEMPORARY VIEW IF NOT EXISTS") { + withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { + withTable("jt2") { + withView("testView") { + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") + + val df = (1 until 10).map(i => i -> i).toDF("i", "j") + df.write.format("json").saveAsTable("jt2") + sql("CREATE TEMPORARY VIEW IF NOT EXISTS testView AS SELECT * FROM jt2") + + // make sure our view doesn't change. + checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i))) + } + } + } + } + + test(s"correctly handle CREATE OR REPLACE TEMPORARY VIEW") { + withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { + withTable("jt2") { + withView("testView") { + sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT id FROM jt") + checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i))) + + val df = (1 until 10).map(i => i -> i).toDF("i", "j") + df.write.format("json").saveAsTable("jt2") + sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT * FROM jt2") + // make sure the view has been changed. + checkAnswer(sql("SELECT * FROM testView ORDER BY i"), (1 to 9).map(i => Row(i, i))) + + sql("DROP VIEW testView") + + val e = intercept[AnalysisException] { + sql("CREATE OR REPLACE TEMPORARY VIEW IF NOT EXISTS testView AS SELECT id FROM jt") + } + assert(e.message.contains("not allowed to define a view")) + } } } } @@ -214,5 +332,4 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { } } } - } From 8af667eda1f6a33774734b5a42ffae62bca22015 Mon Sep 17 00:00:00 2001 From: Sean Zhong Date: Wed, 4 May 2016 00:28:41 +0800 Subject: [PATCH 2/5] remove SQLConf.NATIVE_VIEW conf --- .../sql/hive/execution/SQLViewSuite.scala | 92 ++++++++----------- 1 file changed, 40 insertions(+), 52 deletions(-) diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala index 91cf1c7870a5..431ff94716ef 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala @@ -101,47 +101,39 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { } test("correctly parse CREATE TEMPORARY VIEW statement") { - withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { - withView("testView") { - sql( - """CREATE TEMPORARY VIEW IF NOT EXISTS - |testView (c1 COMMENT 'blabla', c2 COMMENT 'blabla') - |TBLPROPERTIES ('a' = 'b') - |AS SELECT * FROM jt""".stripMargin) - checkAnswer(sql("SELECT c1, c2 FROM testView ORDER BY c1"), (1 to 9).map(i => Row(i, i))) - } + withView("testView") { + sql( + """CREATE TEMPORARY VIEW IF NOT EXISTS + |testView (c1 COMMENT 'blabla', c2 COMMENT 'blabla') + |TBLPROPERTIES ('a' = 'b') + |AS SELECT * FROM jt""".stripMargin) + checkAnswer(sql("SELECT c1, c2 FROM testView ORDER BY c1"), (1 to 9).map(i => Row(i, i))) } } test("should NOT allow CREATE TEMPORARY VIEW when TEMPORARY VIEW with same name exists") { - withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { - withView("testView") { - sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") + withView("testView") { + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") - val e = intercept[AnalysisException] { - sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect() - } - - assert(e.message.contains("Temporary view") && e.message.contains("already exists")) + val e = intercept[AnalysisException] { + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect() } + + assert(e.message.contains("Temporary view") && e.message.contains("already exists")) } } test("should allow CREATE TEMPORARY VIEW when a permanent VIEW with same name exists") { - withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { - withView("testView", "default.testView") { - sql("CREATE VIEW testView AS SELECT id FROM jt") - sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect() - } + withView("testView", "default.testView") { + sql("CREATE VIEW testView AS SELECT id FROM jt") + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect() } } test("should allow CREATE permanent VIEW when a TEMPORARY VIEW with same name exists") { - withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { - withView("testView", "default.testView") { - sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") - sql("CREATE VIEW testView AS SELECT id FROM jt").collect() - } + withView("testView", "default.testView") { + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") + sql("CREATE VIEW testView AS SELECT id FROM jt").collect() } } @@ -163,42 +155,38 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { } test("correctly handle CREATE TEMPORARY VIEW IF NOT EXISTS") { - withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { - withTable("jt2") { - withView("testView") { - sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") + withTable("jt2") { + withView("testView") { + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") - val df = (1 until 10).map(i => i -> i).toDF("i", "j") - df.write.format("json").saveAsTable("jt2") - sql("CREATE TEMPORARY VIEW IF NOT EXISTS testView AS SELECT * FROM jt2") + val df = (1 until 10).map(i => i -> i).toDF("i", "j") + df.write.format("json").saveAsTable("jt2") + sql("CREATE TEMPORARY VIEW IF NOT EXISTS testView AS SELECT * FROM jt2") - // make sure our view doesn't change. - checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i))) - } + // make sure our view doesn't change. + checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i))) } } } test(s"correctly handle CREATE OR REPLACE TEMPORARY VIEW") { - withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") { - withTable("jt2") { - withView("testView") { - sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT id FROM jt") - checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i))) + withTable("jt2") { + withView("testView") { + sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT id FROM jt") + checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i))) - val df = (1 until 10).map(i => i -> i).toDF("i", "j") - df.write.format("json").saveAsTable("jt2") - sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT * FROM jt2") - // make sure the view has been changed. - checkAnswer(sql("SELECT * FROM testView ORDER BY i"), (1 to 9).map(i => Row(i, i))) + val df = (1 until 10).map(i => i -> i).toDF("i", "j") + df.write.format("json").saveAsTable("jt2") + sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT * FROM jt2") + // make sure the view has been changed. + checkAnswer(sql("SELECT * FROM testView ORDER BY i"), (1 to 9).map(i => Row(i, i))) - sql("DROP VIEW testView") + sql("DROP VIEW testView") - val e = intercept[AnalysisException] { - sql("CREATE OR REPLACE TEMPORARY VIEW IF NOT EXISTS testView AS SELECT id FROM jt") - } - assert(e.message.contains("not allowed to define a view")) + val e = intercept[AnalysisException] { + sql("CREATE OR REPLACE TEMPORARY VIEW IF NOT EXISTS testView AS SELECT id FROM jt") } + assert(e.message.contains("not allowed to define a view")) } } } From 238db1290faccd7c2144dfe3c25e320b89f96057 Mon Sep 17 00:00:00 2001 From: Sean Zhong Date: Wed, 4 May 2016 11:09:37 +0800 Subject: [PATCH 3/5] disallow syntax "CREATE TEMPORARY VIEW IF NOT EXISTS" to be consistent with "CREATE TEMPORARY TABLE" syntax --- .../spark/sql/execution/command/views.scala | 24 +++++-------- .../sql/hive/execution/SQLViewSuite.scala | 34 ++++++------------- 2 files changed, 19 insertions(+), 39 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala index aecc6a2fee1d..ca589cf677f8 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala @@ -65,6 +65,12 @@ case class CreateViewCommand( "It is not allowed to define a view with both IF NOT EXISTS and OR REPLACE.") } + // Disallows 'CREATE TEMPORARY VIEW IF NOT EXISTS' to be consistent with 'CREATE TEMPORARY TABLE' + if (allowExisting && isTemporary) { + throw new AnalysisException( + "It is not allowed to define a TEMPORARY view with IF NOT EXISTS.") + } + // Temporary view names should NOT contain database prefix like "database.table" if (isTemporary && tableDesc.identifier.database.isDefined) { val database = tableDesc.identifier.database.get @@ -113,8 +119,8 @@ case class CreateViewCommand( Seq.empty[Row] } - private def createTemporaryView(table: TableIdentifier, sparkSession: SparkSession, - analyzedPlan: LogicalPlan): Unit = { + private def createTemporaryView( + table: TableIdentifier, sparkSession: SparkSession, analyzedPlan: LogicalPlan): Unit = { val sessionState = sparkSession.sessionState val catalog = sessionState.catalog @@ -131,19 +137,7 @@ case class CreateViewCommand( } } - if (catalog.tableExists(table) && catalog.isTemporaryTable(table)) { - if (allowExisting) { - // Does nothing when the target view already exists. - } else if (replace) { - // Replaces the temp view if it exists - catalog.createTempTable(table.table, logicalPlan, replace) - } else { - throw new AnalysisException(s"Temporary view $table already exists. ") - } - } else { - // Creates a temp view - catalog.createTempTable(table.table, logicalPlan, overrideIfExists = false) - } + catalog.createTempTable(table.table, logicalPlan, replace) } /** diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala index 431ff94716ef..7908fb87eaf8 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala @@ -75,6 +75,14 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { assert(e.message.contains("It is not allowed to add database prefix")) } + test("error handling: disallow IF NOT EXISTS for CREATE TEMPORARY VIEW") { + // Full qualified table name is not allowed + val e = intercept[AnalysisException] { + sql("CREATE TEMPORARY VIEW IF NOT EXISTS default.myabcdview AS SELECT * FROM jt").collect() + } + assert(e.message.contains("It is not allowed to define a TEMPORARY view with IF NOT EXISTS")) + } + test("error handling: fail if the temp view sql itself is invalid") { // A table that does not exist for temporary view intercept[AnalysisException] { @@ -103,7 +111,7 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { test("correctly parse CREATE TEMPORARY VIEW statement") { withView("testView") { sql( - """CREATE TEMPORARY VIEW IF NOT EXISTS + """CREATE TEMPORARY VIEW |testView (c1 COMMENT 'blabla', c2 COMMENT 'blabla') |TBLPROPERTIES ('a' = 'b') |AS SELECT * FROM jt""".stripMargin) @@ -119,7 +127,7 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect() } - assert(e.message.contains("Temporary view") && e.message.contains("already exists")) + assert(e.message.contains("Temporary table") && e.message.contains("already exists")) } } @@ -154,21 +162,6 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { } } - test("correctly handle CREATE TEMPORARY VIEW IF NOT EXISTS") { - withTable("jt2") { - withView("testView") { - sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") - - val df = (1 until 10).map(i => i -> i).toDF("i", "j") - df.write.format("json").saveAsTable("jt2") - sql("CREATE TEMPORARY VIEW IF NOT EXISTS testView AS SELECT * FROM jt2") - - // make sure our view doesn't change. - checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i))) - } - } - } - test(s"correctly handle CREATE OR REPLACE TEMPORARY VIEW") { withTable("jt2") { withView("testView") { @@ -180,13 +173,6 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT * FROM jt2") // make sure the view has been changed. checkAnswer(sql("SELECT * FROM testView ORDER BY i"), (1 to 9).map(i => Row(i, i))) - - sql("DROP VIEW testView") - - val e = intercept[AnalysisException] { - sql("CREATE OR REPLACE TEMPORARY VIEW IF NOT EXISTS testView AS SELECT id FROM jt") - } - assert(e.message.contains("not allowed to define a view")) } } } From 1e20bb0df84ffcfe173c234375f8eb0eb7662242 Mon Sep 17 00:00:00 2001 From: Sean Zhong Date: Wed, 4 May 2016 11:46:11 +0800 Subject: [PATCH 4/5] fix indentation --- .../apache/spark/sql/execution/command/views.scala | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala index ca589cf677f8..ee7b0c2e5f20 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala @@ -38,9 +38,9 @@ import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Project} * @param replace if true, and if the view already exists, updates it; if false, and if the view * already exists, throws analysis exception. * @param isTemporary if true, the view is created as a temporary view. Temporary views are dropped - * at the end of current Spark session. Existing permanent relations with the same - * name are not visible to the current session while the temporary view exists, - * unless they are specified with full qualified table name with database prefix. + * at the end of current Spark session. Existing permanent relations with the same + * name are not visible to the current session while the temporary view exists, + * unless they are specified with full qualified table name with database prefix. * @param sql the original sql */ case class CreateViewCommand( @@ -106,9 +106,9 @@ case class CreateViewCommand( } else { // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already // exists. - throw new AnalysisException(s"View $tableIdentifier already exists. " + - "If you want to update the view definition, please use ALTER VIEW AS or " + - "CREATE OR REPLACE VIEW AS") + throw new AnalysisException( + s"View $tableIdentifier already exists. If you want to update the view definition, " + + "please use ALTER VIEW AS or CREATE OR REPLACE VIEW AS") } } else { // Create the view if it doesn't exist. From 85d121cc79e3a17bb5d22eb7000997b4d0f981cc Mon Sep 17 00:00:00 2001 From: Sean Zhong Date: Wed, 4 May 2016 20:26:24 +0800 Subject: [PATCH 5/5] fix style --- .../sql/hive/execution/SQLViewSuite.scala | 39 +++++++++---------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala index 7908fb87eaf8..e1cadd8f51cd 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala @@ -39,18 +39,18 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { test("nested views (interleaved with temporary views)") { withView("jtv1", "jtv2", "jtv3", "temp_jtv1", "temp_jtv2", "temp_jtv3") { - sql("CREATE VIEW jtv1 AS SELECT * FROM jt WHERE id > 3").collect() - sql("CREATE VIEW jtv2 AS SELECT * FROM jtv1 WHERE id < 6").collect() + sql("CREATE VIEW jtv1 AS SELECT * FROM jt WHERE id > 3") + sql("CREATE VIEW jtv2 AS SELECT * FROM jtv1 WHERE id < 6") checkAnswer(sql("select count(*) FROM jtv2"), Row(2)) // Checks temporary views - sql("CREATE TEMPORARY VIEW temp_jtv1 AS SELECT * FROM jt WHERE id > 3").collect() - sql("CREATE TEMPORARY VIEW temp_jtv2 AS SELECT * FROM temp_jtv1 WHERE id < 6").collect() + sql("CREATE TEMPORARY VIEW temp_jtv1 AS SELECT * FROM jt WHERE id > 3") + sql("CREATE TEMPORARY VIEW temp_jtv2 AS SELECT * FROM temp_jtv1 WHERE id < 6") checkAnswer(sql("select count(*) FROM temp_jtv2"), Row(2)) // Checks interleaved temporary view and normal view - sql("CREATE TEMPORARY VIEW temp_jtv3 AS SELECT * FROM jt WHERE id > 3").collect() - sql("CREATE VIEW jtv3 AS SELECT * FROM temp_jtv3 WHERE id < 6").collect() + sql("CREATE TEMPORARY VIEW temp_jtv3 AS SELECT * FROM jt WHERE id > 3") + sql("CREATE VIEW jtv3 AS SELECT * FROM temp_jtv3 WHERE id < 6") checkAnswer(sql("select count(*) FROM jtv3"), Row(2)) } } @@ -68,17 +68,16 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { } test("error handling: fail if the temp view name contains the database prefix") { - // Full qualified table name is not allowed + // Fully qualified table name like "database.table" is not allowed for temporary view val e = intercept[AnalysisException] { - sql("CREATE OR REPLACE TEMPORARY VIEW default.myabcdview AS SELECT * FROM jt").collect() + sql("CREATE OR REPLACE TEMPORARY VIEW default.myabcdview AS SELECT * FROM jt") } assert(e.message.contains("It is not allowed to add database prefix")) } test("error handling: disallow IF NOT EXISTS for CREATE TEMPORARY VIEW") { - // Full qualified table name is not allowed val e = intercept[AnalysisException] { - sql("CREATE TEMPORARY VIEW IF NOT EXISTS default.myabcdview AS SELECT * FROM jt").collect() + sql("CREATE TEMPORARY VIEW IF NOT EXISTS myabcdview AS SELECT * FROM jt") } assert(e.message.contains("It is not allowed to define a TEMPORARY view with IF NOT EXISTS")) } @@ -87,12 +86,11 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { // A table that does not exist for temporary view intercept[AnalysisException] { sql("CREATE OR REPLACE TEMPORARY VIEW myabcdview AS SELECT * FROM table_not_exist1345") - .collect() } // A column that does not exist, for temporary view intercept[AnalysisException] { - sql("CREATE OR REPLACE TEMPORARY VIEW myabcdview AS SELECT random1234 FROM jt").collect() + sql("CREATE OR REPLACE TEMPORARY VIEW myabcdview AS SELECT random1234 FROM jt") } } @@ -112,9 +110,10 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { withView("testView") { sql( """CREATE TEMPORARY VIEW - |testView (c1 COMMENT 'blabla', c2 COMMENT 'blabla') - |TBLPROPERTIES ('a' = 'b') - |AS SELECT * FROM jt""".stripMargin) + |testView (c1 COMMENT 'blabla', c2 COMMENT 'blabla') + |TBLPROPERTIES ('a' = 'b') + |AS SELECT * FROM jt + |""".stripMargin) checkAnswer(sql("SELECT c1, c2 FROM testView ORDER BY c1"), (1 to 9).map(i => Row(i, i))) } } @@ -124,7 +123,7 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") val e = intercept[AnalysisException] { - sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect() + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") } assert(e.message.contains("Temporary table") && e.message.contains("already exists")) @@ -134,14 +133,14 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { test("should allow CREATE TEMPORARY VIEW when a permanent VIEW with same name exists") { withView("testView", "default.testView") { sql("CREATE VIEW testView AS SELECT id FROM jt") - sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect() + sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt") } } test("should allow CREATE permanent VIEW when a TEMPORARY 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").collect() + sql("CREATE VIEW testView AS SELECT id FROM jt") } } @@ -168,9 +167,7 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton { sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT id FROM jt") checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i))) - val df = (1 until 10).map(i => i -> i).toDF("i", "j") - df.write.format("json").saveAsTable("jt2") - sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT * FROM jt2") + sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT id AS i, id AS j FROM jt") // make sure the view has been changed. checkAnswer(sql("SELECT * FROM testView ORDER BY i"), (1 to 9).map(i => Row(i, i))) }