This is an automated email from the ASF dual-hosted git repository.
gengliang 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 47726027867a [SPARK-48929] Fix view internal error and clean up parser
exception context
47726027867a is described below
commit 47726027867ade03282f9c77312afc7b88fda440
Author: Serge Rielau <[email protected]>
AuthorDate: Mon Jul 22 11:24:28 2024 -0700
[SPARK-48929] Fix view internal error and clean up parser exception context
### What changes were proposed in this pull request?
Replace INVALID_VIEW_TEXT (SQLSTATE: XX000) error-condition which is forced
when view expansion fails to parse with the underlying error.
The reason being that the underlying error has important information AND it
is much more likely that view expansion fails because of a behavior change in
the parser than some nefarious action or simple corruption.
For example the recent removal of `!` and `NOT` equivalence fails old view
on on Spark 4.0 if they exploit the unsupported syntax.
As part of this change we also include the Query context summary in the
ParserException processing.
This will expose the view name along side the view query error message.
### Why are the changes needed?
Better error handling improving ability of customers to do root cause
analysis.
### Does this PR introduce _any_ user-facing change?
We are now giving more error context in case of parer errors and
specifically in case of parser errors in view expansion
### How was this patch tested?
Existing QA
### Was this patch authored or co-authored using generative AI tooling?
No
Closes #47405 from srielau/SPARK-48929-Fix-view-intrenal-error.
Authored-by: Serge Rielau <[email protected]>
Signed-off-by: Gengliang Wang <[email protected]>
---
.../src/main/resources/error/error-conditions.json | 6 ----
.../apache/spark/sql/catalyst/parser/parsers.scala | 35 +++++++++++++---------
.../sql/catalyst/catalog/SessionCatalog.scala | 11 ++-----
.../spark/sql/errors/QueryCompilationErrors.scala | 8 -----
.../catalyst/parser/SqlScriptingParserSuite.scala | 2 +-
.../spark/sql/execution/SQLViewTestSuite.scala | 12 ++++++--
6 files changed, 34 insertions(+), 40 deletions(-)
diff --git a/common/utils/src/main/resources/error/error-conditions.json
b/common/utils/src/main/resources/error/error-conditions.json
index 84681ab8c225..d8edc89ba83e 100644
--- a/common/utils/src/main/resources/error/error-conditions.json
+++ b/common/utils/src/main/resources/error/error-conditions.json
@@ -2983,12 +2983,6 @@
],
"sqlState" : "22023"
},
- "INVALID_VIEW_TEXT" : {
- "message" : [
- "The view <viewName> cannot be displayed due to invalid view text:
<viewText>. This may be caused by an unauthorized modification of the view or
an incorrect query syntax. Please check your query syntax and verify that the
view has not been tampered with."
- ],
- "sqlState" : "XX000"
- },
"INVALID_WHERE_CONDITION" : {
"message" : [
"The WHERE condition <condition> contains invalid expressions:
<expressionList>.",
diff --git
a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/parser/parsers.scala
b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/parser/parsers.scala
index b366d8b78743..0b9e6ea021be 100644
--- a/sql/api/src/main/scala/org/apache/spark/sql/catalyst/parser/parsers.scala
+++ b/sql/api/src/main/scala/org/apache/spark/sql/catalyst/parser/parsers.scala
@@ -249,20 +249,27 @@ class ParseException private(
override def getMessage: String = {
val builder = new StringBuilder
builder ++= "\n" ++= message
- start match {
- case Origin(Some(l), Some(p), _, _, _, _, _, _, _) =>
- builder ++= s" (line $l, pos $p)\n"
- command.foreach { cmd =>
- val (above, below) = cmd.split("\n").splitAt(l)
- builder ++= "\n== SQL ==\n"
- above.foreach(builder ++= _ += '\n')
- builder ++= (0 until p).map(_ => "-").mkString("") ++= "^^^\n"
- below.foreach(builder ++= _ += '\n')
- }
- case _ =>
- command.foreach { cmd =>
- builder ++= "\n== SQL ==\n" ++= cmd
- }
+ if (queryContext.nonEmpty) {
+ builder ++= "\n"
+ queryContext.foreach { ctx =>
+ builder ++= ctx.summary()
+ }
+ } else {
+ start match {
+ case Origin(Some(l), Some(p), _, _, _, _, _, _, _) =>
+ builder ++= s" (line $l, pos $p)\n"
+ command.foreach { cmd =>
+ val (above, below) = cmd.split("\n").splitAt(l)
+ builder ++= "\n== SQL ==\n"
+ above.foreach(builder ++= _ += '\n')
+ builder ++= (0 until p).map(_ => "-").mkString("") ++= "^^^\n"
+ below.foreach(builder ++= _ += '\n')
+ }
+ case _ =>
+ command.foreach { cmd =>
+ builder ++= "\n== SQL ==\n" ++= cmd
+ }
+ }
}
builder.toString
}
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
index fa271eee73d0..701c68684c34 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala
@@ -36,9 +36,9 @@ import org.apache.spark.sql.catalyst._
import org.apache.spark.sql.catalyst.analysis._
import org.apache.spark.sql.catalyst.analysis.FunctionRegistry.FunctionBuilder
import org.apache.spark.sql.catalyst.expressions.{Alias, Cast, Expression,
ExpressionInfo, NamedExpression, UpCast}
-import org.apache.spark.sql.catalyst.parser.{CatalystSqlParser,
ParseException, ParserInterface}
+import org.apache.spark.sql.catalyst.parser.{CatalystSqlParser,
ParserInterface}
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Project,
SubqueryAlias, View}
-import org.apache.spark.sql.catalyst.trees.{CurrentOrigin, Origin}
+import org.apache.spark.sql.catalyst.trees.CurrentOrigin
import org.apache.spark.sql.catalyst.util.{CharVarcharUtils, StringUtils}
import org.apache.spark.sql.connector.catalog.CatalogManager
import org.apache.spark.sql.errors.{QueryCompilationErrors,
QueryExecutionErrors}
@@ -962,19 +962,14 @@ class SessionCatalog(
throw SparkException.internalError("Invalid view without text.")
}
val viewConfigs = metadata.viewSQLConfigs
- val origin = Origin(
+ val origin = CurrentOrigin.get.copy(
objectType = Some("VIEW"),
objectName = Some(metadata.qualifiedName)
)
val parsedPlan =
SQLConf.withExistingConf(View.effectiveSQLConf(viewConfigs, isTempView)) {
- try {
CurrentOrigin.withOrigin(origin) {
parser.parseQuery(viewText)
}
- } catch {
- case _: ParseException =>
- throw QueryCompilationErrors.invalidViewText(viewText,
metadata.qualifiedName)
- }
}
val schemaMode = metadata.viewSchemaMode
if (schemaMode == SchemaEvolution) {
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 b489387c7868..9de4fa2f6b26 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
@@ -3464,14 +3464,6 @@ private[sql] object QueryCompilationErrors extends
QueryErrorsBase with Compilat
messageParameters = Map("errorMessage" -> errorMessage))
}
- def invalidViewText(viewText: String, viewName: String): Throwable = {
- new AnalysisException(
- errorClass = "INVALID_VIEW_TEXT",
- messageParameters = Map(
- "viewText" -> viewText,
- "viewName" -> toSQLId(viewName)))
- }
-
def invalidTimeTravelSpecError(): Throwable = {
new AnalysisException(
errorClass = "INVALID_TIME_TRAVEL_SPEC",
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/SqlScriptingParserSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/SqlScriptingParserSuite.scala
index d4eb5fd747ac..3e558ae5f977 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/SqlScriptingParserSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/parser/SqlScriptingParserSuite.scala
@@ -48,7 +48,7 @@ class SqlScriptingParserSuite extends SparkFunSuite with
SQLHelper {
}
assert(e.getErrorClass === "PARSE_SYNTAX_ERROR")
assert(e.getMessage.contains("Syntax error"))
- assert(e.getMessage.contains("SELECT 1 SELECT 1"))
+ assert(e.getMessage.contains("SELECT"))
}
test("multi select") {
diff --git
a/sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewTestSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewTestSuite.scala
index e75413b804f4..aa6295fa8815 100644
---
a/sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewTestSuite.scala
+++
b/sql/core/src/test/scala/org/apache/spark/sql/execution/SQLViewTestSuite.scala
@@ -724,9 +724,15 @@ class PersistedViewTestSuite extends SQLViewTestSuite with
SharedSparkSession {
exception = intercept[AnalysisException] {
sql("SELECT * FROM v")
},
- errorClass = "INVALID_VIEW_TEXT",
- parameters = Map(
- "viewText" -> "DROP VIEW v", "viewName" ->
tableIdentifier("v").quotedString)
+ errorClass = "PARSE_SYNTAX_ERROR",
+ parameters = Map("error" -> "'DROP'", "hint" -> ""),
+ context = ExpectedContext(
+ objectType = "VIEW",
+ objectName = "spark_catalog.default.v",
+ startIndex = 14,
+ stopIndex = 14,
+ fragment = "v"
+ )
)
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
