You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2021/10/19 06:40:15 UTC
[spark] branch master updated: [SPARK-36871][SQL][FOLLOWUP] Move
error checking from create cmd to parser
This is an automated email from the ASF dual-hosted git repository.
wenchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new ebca5232 [SPARK-36871][SQL][FOLLOWUP] Move error checking from create cmd to parser
ebca5232 is described below
commit ebca5232811cb0701d4062ac7ddc21fccc936490
Author: Huaxin Gao <hu...@apple.com>
AuthorDate: Tue Oct 19 14:38:49 2021 +0800
[SPARK-36871][SQL][FOLLOWUP] Move error checking from create cmd to parser
### What changes were proposed in this pull request?
Move error checking from create cmd to parser
### Why are the changes needed?
catch error earlier and also make code consistent between parsing CreateFunction and CreateView
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Existing tests
Closes #34283 from huaxingao/create_view_followup.
Authored-by: Huaxin Gao <hu...@apple.com>
Signed-off-by: Wenchen Fan <we...@databricks.com>
---
.../spark/sql/errors/QueryCompilationErrors.scala | 26 ---------------
.../spark/sql/errors/QueryParsingErrors.scala | 34 +++++++++++++++++++
.../spark/sql/execution/SparkSqlParser.scala | 39 +++++++++++++++++++---
.../spark/sql/execution/command/functions.scala | 9 -----
.../apache/spark/sql/execution/command/views.scala | 21 +-----------
5 files changed, 69 insertions(+), 60 deletions(-)
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
index 385e6b7..eb8985d 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
@@ -1849,19 +1849,6 @@ object QueryCompilationErrors {
new AnalysisException("Cannot overwrite a path that is also being read from.")
}
- def createFuncWithBothIfNotExistsAndReplaceError(): Throwable = {
- new AnalysisException("CREATE FUNCTION with both IF NOT EXISTS and REPLACE is not allowed.")
- }
-
- def defineTempFuncWithIfNotExistsError(): Throwable = {
- new AnalysisException("It is not allowed to define a TEMPORARY function with IF NOT EXISTS.")
- }
-
- def specifyingDBInCreateTempFuncError(databaseName: String): Throwable = {
- new AnalysisException(
- s"Specifying a database in CREATE TEMPORARY FUNCTION is not allowed: '$databaseName'")
- }
-
def specifyingDBInDropTempFuncError(databaseName: String): Throwable = {
new AnalysisException(
s"Specifying a database in DROP TEMPORARY FUNCTION is not allowed: '$databaseName'")
@@ -2011,19 +1998,6 @@ object QueryCompilationErrors {
features.map(" - " + _).mkString("\n"))
}
- def createViewWithBothIfNotExistsAndReplaceError(): Throwable = {
- new AnalysisException("CREATE VIEW with both IF NOT EXISTS and REPLACE is not allowed.")
- }
-
- def defineTempViewWithIfNotExistsError(): Throwable = {
- new AnalysisException("It is not allowed to define a TEMPORARY view with IF NOT EXISTS.")
- }
-
- def notAllowedToAddDBPrefixForTempViewError(database: String): Throwable = {
- new AnalysisException(
- s"It is not allowed to add database prefix `$database` for the TEMPORARY view name.")
- }
-
def logicalPlanForViewNotAnalyzedError(): Throwable = {
new AnalysisException("The logical plan that represents the view is not analyzed.")
}
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala
index 3af63f1..090f73d 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala
@@ -391,4 +391,38 @@ object QueryParsingErrors {
def invalidGroupingSetError(element: String, ctx: GroupingAnalyticsContext): Throwable = {
new ParseException(s"Empty set in $element grouping sets is not supported.", ctx)
}
+
+ def createViewWithBothIfNotExistsAndReplaceError(ctx: CreateViewContext): Throwable = {
+ new ParseException("CREATE VIEW with both IF NOT EXISTS and REPLACE is not allowed.", ctx)
+ }
+
+ def defineTempViewWithIfNotExistsError(ctx: CreateViewContext): Throwable = {
+ new ParseException("It is not allowed to define a TEMPORARY view with IF NOT EXISTS.", ctx)
+ }
+
+ def notAllowedToAddDBPrefixForTempViewError(
+ database: String,
+ ctx: CreateViewContext): Throwable = {
+ new ParseException(
+ s"It is not allowed to add database prefix `$database` for the TEMPORARY view name.", ctx)
+ }
+
+ def createFuncWithBothIfNotExistsAndReplaceError(ctx: CreateFunctionContext): Throwable = {
+ new ParseException("CREATE FUNCTION with both IF NOT EXISTS and REPLACE is not allowed.", ctx)
+ }
+
+ def defineTempFuncWithIfNotExistsError(ctx: CreateFunctionContext): Throwable = {
+ new ParseException("It is not allowed to define a TEMPORARY function with IF NOT EXISTS.", ctx)
+ }
+
+ def unsupportedFunctionNameError(quoted: String, ctx: CreateFunctionContext): Throwable = {
+ new ParseException(s"Unsupported function name '$quoted'", ctx)
+ }
+
+ def specifyingDBInCreateTempFuncError(
+ databaseName: String,
+ ctx: CreateFunctionContext): Throwable = {
+ new ParseException(
+ s"Specifying a database in CREATE TEMPORARY FUNCTION is not allowed: '$databaseName'", ctx)
+ }
}
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 34eacb2..d26b261 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
@@ -34,7 +34,7 @@ import org.apache.spark.sql.catalyst.parser._
import org.apache.spark.sql.catalyst.parser.SqlBaseParser._
import org.apache.spark.sql.catalyst.plans.logical._
import org.apache.spark.sql.catalyst.util.DateTimeConstants
-import org.apache.spark.sql.errors.{QueryCompilationErrors, QueryParsingErrors}
+import org.apache.spark.sql.errors.QueryParsingErrors
import org.apache.spark.sql.execution.command._
import org.apache.spark.sql.execution.datasources._
import org.apache.spark.sql.internal.{HiveSerDe, SQLConf, VariableSubstitution}
@@ -460,6 +460,10 @@ class SparkSqlAstBuilder extends AstBuilder {
}
}
+ if (ctx.EXISTS != null && ctx.REPLACE != null) {
+ throw QueryParsingErrors.createViewWithBothIfNotExistsAndReplaceError(ctx)
+ }
+
val properties = ctx.tablePropertyList.asScala.headOption.map(visitPropertyKeyValues)
.getOrElse(Map.empty)
if (ctx.TEMPORARY != null && !properties.isEmpty) {
@@ -474,6 +478,9 @@ class SparkSqlAstBuilder extends AstBuilder {
LocalTempView
}
if (viewType == PersistedView) {
+ val originalText = source(ctx.query)
+ assert(Option(originalText).isDefined,
+ "'originalText' must be provided to create permanent view")
CreateView(
UnresolvedDBObjectName(
visitMultipartIdentifier(ctx.multipartIdentifier),
@@ -481,13 +488,26 @@ class SparkSqlAstBuilder extends AstBuilder {
userSpecifiedColumns,
visitCommentSpecList(ctx.commentSpec()),
properties,
- Option(source(ctx.query)),
+ Some(originalText),
plan(ctx.query),
ctx.EXISTS != null,
ctx.REPLACE != null)
} else {
+ // Disallows 'CREATE TEMPORARY VIEW IF NOT EXISTS' to be consistent with
+ // 'CREATE TEMPORARY TABLE'
+ if (ctx.EXISTS != null) {
+ throw QueryParsingErrors.defineTempViewWithIfNotExistsError(ctx)
+ }
+
+ val tableIdentifier = visitMultipartIdentifier(ctx.multipartIdentifier).asTableIdentifier
+ if (tableIdentifier.database.isDefined) {
+ // Temporary view names should NOT contain database prefix like "database.table"
+ throw QueryParsingErrors
+ .notAllowedToAddDBPrefixForTempViewError(tableIdentifier.database.get, ctx)
+ }
+
CreateViewCommand(
- visitMultipartIdentifier(ctx.multipartIdentifier).asTableIdentifier,
+ tableIdentifier,
userSpecifiedColumns,
visitCommentSpecList(ctx.commentSpec()),
properties,
@@ -519,6 +539,10 @@ class SparkSqlAstBuilder extends AstBuilder {
}
}
+ if (ctx.EXISTS != null && ctx.REPLACE != null) {
+ throw QueryParsingErrors.createFuncWithBothIfNotExistsAndReplaceError(ctx)
+ }
+
val functionIdentifier = visitMultipartIdentifier(ctx.multipartIdentifier)
if (ctx.TEMPORARY == null) {
CreateFunction(
@@ -530,11 +554,16 @@ class SparkSqlAstBuilder extends AstBuilder {
ctx.EXISTS != null,
ctx.REPLACE != null)
} else {
+ // Disallow to define a temporary function with `IF NOT EXISTS`
+ if (ctx.EXISTS != null) {
+ throw QueryParsingErrors.defineTempFuncWithIfNotExistsError(ctx)
+ }
+
if (functionIdentifier.length > 2) {
- throw QueryCompilationErrors.unsupportedFunctionNameError(functionIdentifier.quoted)
+ throw QueryParsingErrors.unsupportedFunctionNameError(functionIdentifier.quoted, ctx)
} else if (functionIdentifier.length == 2) {
// Temporary function names should not contain database prefix like "database.function"
- throw QueryCompilationErrors.specifyingDBInCreateTempFuncError(functionIdentifier.head)
+ throw QueryParsingErrors.specifyingDBInCreateTempFuncError(functionIdentifier.head, ctx)
}
CreateFunctionCommand(
None,
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala
index f39df38..09aa569 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/command/functions.scala
@@ -57,15 +57,6 @@ case class CreateFunctionCommand(
replace: Boolean)
extends LeafRunnableCommand {
- if (ignoreIfExists && replace) {
- throw QueryCompilationErrors.createFuncWithBothIfNotExistsAndReplaceError()
- }
-
- // Disallow to define a temporary function with `IF NOT EXISTS`
- if (ignoreIfExists && isTemp) {
- throw QueryCompilationErrors.defineTempFuncWithIfNotExistsError()
- }
-
override def run(sparkSession: SparkSession): Seq[Row] = {
val catalog = sparkSession.sessionState.catalog
val func = CatalogFunction(FunctionIdentifier(functionName, databaseName), className, resources)
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 2eb5d76..552bf44 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
@@ -25,7 +25,7 @@ import org.json4s.jackson.JsonMethods._
import org.apache.spark.internal.Logging
import org.apache.spark.sql.{Row, SparkSession}
import org.apache.spark.sql.catalyst.{FunctionIdentifier, SQLConfHelper, TableIdentifier}
-import org.apache.spark.sql.catalyst.analysis.{GlobalTempView, LocalTempView, PersistedView, ViewType}
+import org.apache.spark.sql.catalyst.analysis.{GlobalTempView, LocalTempView, ViewType}
import org.apache.spark.sql.catalyst.catalog.{CatalogStorageFormat, CatalogTable, CatalogTableType, SessionCatalog, TemporaryViewRelation}
import org.apache.spark.sql.catalyst.expressions.{Alias, Attribute, SubqueryExpression, UserDefinedExpression}
import org.apache.spark.sql.catalyst.plans.logical.{AnalysisOnlyCommand, LogicalPlan, Project, View}
@@ -82,27 +82,8 @@ case class CreateViewCommand(
def markAsAnalyzed(): LogicalPlan = copy(isAnalyzed = true)
- if (viewType == PersistedView) {
- require(originalText.isDefined, "'originalText' must be provided to create permanent view")
- }
-
- if (allowExisting && replace) {
- throw QueryCompilationErrors.createViewWithBothIfNotExistsAndReplaceError()
- }
-
private def isTemporary = viewType == LocalTempView || viewType == GlobalTempView
- // Disallows 'CREATE TEMPORARY VIEW IF NOT EXISTS' to be consistent with 'CREATE TEMPORARY TABLE'
- if (allowExisting && isTemporary) {
- throw QueryCompilationErrors.defineTempViewWithIfNotExistsError()
- }
-
- // Temporary view names should NOT contain database prefix like "database.table"
- if (isTemporary && name.database.isDefined) {
- val database = name.database.get
- throw QueryCompilationErrors.notAllowedToAddDBPrefixForTempViewError(database)
- }
-
override def run(sparkSession: SparkSession): Seq[Row] = {
if (!isAnalyzed) {
throw QueryCompilationErrors.logicalPlanForViewNotAnalyzedError()
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org