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 <huaxin_...@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 <huaxin_...@apple.com> Signed-off-by: Wenchen Fan <wenc...@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-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org