From 54f3f7041d6e057b6cd9f8d5ec6c71bc4844fdd5 Mon Sep 17 00:00:00 2001 From: RoryQi <1242949407@qq.com> Date: Mon, 5 Jul 2021 01:43:50 +0800 Subject: [PATCH 1/3] [SPARK-36011][SQL][3.0] Disallow altering permanent views based on temporary views or UDFs --- .../spark/sql/execution/command/views.scala | 76 ++++++++++--------- .../spark/sql/execution/SQLViewSuite.scala | 16 ++++ .../sql/hive/execution/HiveSQLViewSuite.scala | 25 ++++++ 3 files changed, 82 insertions(+), 35 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 ebba726d4b3b..a4a7d32e8b50 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 @@ -107,7 +107,7 @@ case class CreateViewCommand( // When creating a permanent view, not allowed to reference temporary objects. // This should be called after `qe.assertAnalyzed()` (i.e., `child` can be resolved) - verifyTemporaryObjectsNotExists(catalog) + verifyTemporaryObjectsNotExists(catalog, isTemporary, name, child) if (viewType == LocalTempView) { val aliasedPlan = aliasPlan(sparkSession, analyzedPlan) @@ -161,39 +161,7 @@ case class CreateViewCommand( Seq.empty[Row] } - /** - * Permanent views are not allowed to reference temp objects, including temp function and views - */ - private def verifyTemporaryObjectsNotExists(catalog: SessionCatalog): Unit = { - import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ - if (!isTemporary) { - // This func traverses the unresolved plan `child`. Below are the reasons: - // 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding - // logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is - // added/generated from a temporary view. - // 2) The temp functions are represented by multiple classes. Most are inaccessible from this - // package (e.g., HiveGenericUDF). - def verify(child: LogicalPlan) { - child.collect { - // Disallow creating permanent views based on temporary views. - case UnresolvedRelation(nameParts) if catalog.isTempView(nameParts) => - throw new AnalysisException(s"Not allowed to create a permanent view $name by " + - s"referencing a temporary view ${nameParts.quoted}. " + - "Please create a temp view instead by CREATE TEMP VIEW") - case w: With if !w.resolved => w.innerChildren.foreach(verify) - case other if !other.resolved => other.expressions.flatMap(_.collect { - // Traverse subquery plan for any unresolved relations. - case e: SubqueryExpression => verify(e.plan) - // Disallow creating permanent views based on temporary UDFs. - case e: UnresolvedFunction if catalog.isTemporaryFunction(e.name) => - throw new AnalysisException(s"Not allowed to create a permanent view $name by " + - s"referencing a temporary function `${e.name}`") - }) - } - } - verify(child) - } - } + /** * If `userSpecifiedColumns` is defined, alias the analyzed plan to the user specified columns, @@ -266,7 +234,8 @@ case class AlterViewAsCommand( val qe = session.sessionState.executePlan(query) qe.assertAnalyzed() val analyzedPlan = qe.analyzed - + val isTemporary = session.sessionState.catalog.isTemporaryTable(name) + verifyTemporaryObjectsNotExists(session.sessionState.catalog, isTemporary, name, query) if (session.sessionState.catalog.alterTempViewDefinition(name, analyzedPlan)) { // a local/global temp view has been altered, we are done. } else { @@ -441,4 +410,41 @@ object ViewHelper { } } } + + /** + * Permanent views are not allowed to reference temp objects, including temp function and views + */ + def verifyTemporaryObjectsNotExists(catalog: SessionCatalog, + isTemporary: Boolean, + name: TableIdentifier, + child: LogicalPlan): Unit = { + import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._ + if (!isTemporary) { + // This func traverses the unresolved plan `child`. Below are the reasons: + // 1) Analyzer replaces unresolved temporary views by a SubqueryAlias with the corresponding + // logical plan. After replacement, it is impossible to detect whether the SubqueryAlias is + // added/generated from a temporary view. + // 2) The temp functions are represented by multiple classes. Most are inaccessible from this + // package (e.g., HiveGenericUDF). + def verify(child: LogicalPlan) { + child.collect { + // Disallow creating permanent views based on temporary views. + case UnresolvedRelation(nameParts) if catalog.isTempView(nameParts) => + throw new AnalysisException(s"Not allowed to create a permanent view $name by " + + s"referencing a temporary view ${nameParts.quoted}. " + + "Please create a temp view instead by CREATE TEMP VIEW") + case w: With if !w.resolved => w.innerChildren.foreach(verify) + case other if !other.resolved => other.expressions.flatMap(_.collect { + // Traverse subquery plan for any unresolved relations. + case e: SubqueryExpression => verify(e.plan) + // Disallow creating permanent views based on temporary UDFs. + case e: UnresolvedFunction if catalog.isTemporaryFunction(e.name) => + throw new AnalysisException(s"Not allowed to create a permanent view $name by " + + s"referencing a temporary function `${e.name}`") + }) + } + } + verify(child) + } + } } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewSuite.scala index 68d9460b592b..9218fc2bbe91 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewSuite.scala @@ -17,6 +17,8 @@ package org.apache.spark.sql.execution +import org.scalatest.Assertions.intercept + import org.apache.spark.sql._ import org.apache.spark.sql.catalyst.TableIdentifier import org.apache.spark.sql.catalyst.analysis.NoSuchTableException @@ -795,4 +797,18 @@ abstract class SQLViewSuite extends QueryTest with SQLTestUtils { } } } + + test("SPARK-36011: Disallow altering permanent views based on temporary views") { + withView("jtv1") { + withTempView("jtv2") { + sql(s"CREATE VIEW jtv1 AS SELECT * FROM jt WHERE id > 3") + sql(s"CREATE TEMPORARY VIEW jtv2 AS SELECT * FROM jt where id < 3") + val e = intercept[AnalysisException] { + sql(s"ALTER VIEW jtv1 AS SELECT * FROM jtv2") + }.getMessage + assert(e.contains("Not allowed to create a permanent view `default`.`jtv1` by " + + "referencing a temporary view jtv2")) + } + } + } } diff --git a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveSQLViewSuite.scala b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveSQLViewSuite.scala index fa43ff14fd79..eb99c708bda0 100644 --- a/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveSQLViewSuite.scala +++ b/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveSQLViewSuite.scala @@ -17,6 +17,8 @@ package org.apache.spark.sql.hive.execution +import org.scalatest.Assertions.intercept + import org.apache.spark.sql.{AnalysisException, Row, SaveMode, SparkSession} import org.apache.spark.sql.catalyst.TableIdentifier import org.apache.spark.sql.catalyst.catalog.{CatalogStorageFormat, CatalogTable, CatalogTableType} @@ -137,4 +139,27 @@ class HiveSQLViewSuite extends SQLViewSuite with TestHiveSingleton { } } } + + test("SPARK-36011: Disallow altering permanent views based on temporary UDFs") { + val tempFunctionName = "temp" + val functionClass = + classOf[org.apache.hadoop.hive.ql.udf.generic.GenericUDFUpper].getCanonicalName + withUserDefinedFunction(tempFunctionName -> true) { + sql(s"CREATE TEMPORARY FUNCTION $tempFunctionName AS '$functionClass'") + withView("view1") { + withTempView("tempView1") { + withTable("tab1") { + (1 to 10).map(i => s"$i").toDF("id").write.saveAsTable("tab1") + sql("CREATE VIEW view1 AS SELECT id from tab1") + + val e = intercept[AnalysisException] { + sql(s"ALTER VIEW view1 AS SELECT $tempFunctionName(id) from tab1") + }.getMessage + assert(e.contains("Not allowed to create a permanent view `default`.`view1` by " + + s"referencing a temporary function `$tempFunctionName`")) + } + } + } + } + } } From 783db4671ae9f16c98723fa9b5b1512847db38ff Mon Sep 17 00:00:00 2001 From: RoryQi <1242949407@qq.com> Date: Wed, 7 Jul 2021 00:57:00 +0800 Subject: [PATCH 2/3] trigger test From 39c2f6eaf67ee13bd1b1e0dcfd5d8f9689108b94 Mon Sep 17 00:00:00 2001 From: RoryQi <1242949407@qq.com> Date: Wed, 7 Jul 2021 11:18:14 +0800 Subject: [PATCH 3/3] trigger test