Repository: spark Updated Branches: refs/heads/master dc4c60098 -> aa412c55e
[SPARK-18259][SQL] Do not capture Throwable in QueryExecution ## What changes were proposed in this pull request? `QueryExecution.toString` currently captures `java.lang.Throwable`s; this is far from a best practice and can lead to confusing situation or invalid application states. This PR fixes this by only capturing `AnalysisException`s. ## How was this patch tested? Added a `QueryExecutionSuite`. Author: Herman van Hovell <hvanhov...@databricks.com> Closes #15760 from hvanhovell/SPARK-18259. Project: http://git-wip-us.apache.org/repos/asf/spark/repo Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/aa412c55 Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/aa412c55 Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/aa412c55 Branch: refs/heads/master Commit: aa412c55e31e61419d3de57ef4b13e50f9b38af0 Parents: dc4c600 Author: Herman van Hovell <hvanhov...@databricks.com> Authored: Thu Nov 3 21:59:59 2016 -0700 Committer: Reynold Xin <r...@databricks.com> Committed: Thu Nov 3 21:59:59 2016 -0700 ---------------------------------------------------------------------- .../spark/sql/execution/QueryExecution.scala | 2 +- .../sql/execution/QueryExecutionSuite.scala | 50 ++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/spark/blob/aa412c55/sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala b/sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala index cb45a6d..b3ef29f 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala @@ -104,7 +104,7 @@ class QueryExecution(val sparkSession: SparkSession, val logical: LogicalPlan) { ReuseSubquery(sparkSession.sessionState.conf)) protected def stringOrError[A](f: => A): String = - try f.toString catch { case e: Throwable => e.toString } + try f.toString catch { case e: AnalysisException => e.toString } /** http://git-wip-us.apache.org/repos/asf/spark/blob/aa412c55/sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala ---------------------------------------------------------------------- diff --git a/sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala new file mode 100644 index 0000000..8bceab3 --- /dev/null +++ b/sql/core/src/test/scala/org/apache/spark/sql/execution/QueryExecutionSuite.scala @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.spark.sql.execution + +import org.apache.spark.sql.AnalysisException +import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, OneRowRelation} +import org.apache.spark.sql.test.SharedSQLContext + +class QueryExecutionSuite extends SharedSQLContext { + test("toString() exception/error handling") { + val badRule = new SparkStrategy { + var mode: String = "" + override def apply(plan: LogicalPlan): Seq[SparkPlan] = mode.toLowerCase match { + case "exception" => throw new AnalysisException(mode) + case "error" => throw new Error(mode) + case _ => Nil + } + } + spark.experimental.extraStrategies = badRule :: Nil + + def qe: QueryExecution = new QueryExecution(spark, OneRowRelation) + + // Nothing! + badRule.mode = "" + assert(qe.toString.contains("OneRowRelation")) + + // Throw an AnalysisException - this should be captured. + badRule.mode = "exception" + assert(qe.toString.contains("org.apache.spark.sql.AnalysisException")) + + // Throw an Error - this should not be captured. + badRule.mode = "error" + val error = intercept[Error](qe.toString) + assert(error.getMessage.contains("error")) + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org