From 690035a877d21de75310011eeecc80f2ff87b4bf Mon Sep 17 00:00:00 2001 From: zhoukang Date: Tue, 17 Jul 2018 18:07:21 +0800 Subject: [PATCH 01/11] [SPARK-24544][SQL] Print actual failure cause when look up function failed --- .../sql/catalyst/analysis/NoSuchItemException.scala | 5 +++-- .../spark/sql/catalyst/catalog/SessionCatalog.scala | 5 +++-- .../org/apache/spark/sql/hive/HiveSessionCatalog.scala | 9 ++++++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala index f5aae60431c15..3866dedceed40 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala @@ -40,10 +40,11 @@ class NoSuchPartitionException( class NoSuchPermanentFunctionException(db: String, func: String) extends AnalysisException(s"Function '$func' not found in database '$db'") -class NoSuchFunctionException(db: String, func: String) +class NoSuchFunctionException(db: String, func: String, rootCause: Option[String] = None) extends AnalysisException( s"Undefined function: '$func'. This function is neither a registered temporary function nor " + - s"a permanent function registered in the database '$db'.") + s"a permanent function registered in the database '$db'." + + s"Exception thrown during look up: ${rootCause.getOrElse("None")}") class NoSuchPartitionsException(db: String, table: String, specs: Seq[TablePartitionSpec]) extends AnalysisException( 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 b09b81eabf60d..f73b5ba471e83 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 @@ -1209,9 +1209,10 @@ class SessionCatalog( databaseExists(db) && externalCatalog.functionExists(db, name.funcName) } - protected def failFunctionLookup(name: FunctionIdentifier): Nothing = { + protected def failFunctionLookup( + name: FunctionIdentifier, rootCause: Option[String] = None): Nothing = { throw new NoSuchFunctionException( - db = name.database.getOrElse(getCurrentDatabase), func = name.funcName) + db = name.database.getOrElse(getCurrentDatabase), func = name.funcName, rootCause) } /** diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala index de41bb418181d..6d46bf490a33b 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala @@ -36,6 +36,7 @@ import org.apache.spark.sql.catalyst.parser.ParserInterface import org.apache.spark.sql.hive.HiveShim.HiveFunctionWrapper import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.types.{DecimalType, DoubleType} +import org.apache.spark.util.{Utils => SparkUtils} private[sql] class HiveSessionCatalog( @@ -141,8 +142,10 @@ private[sql] class HiveSessionCatalog( // built-in function. // Hive is case insensitive. val functionName = funcName.unquotedString.toLowerCase(Locale.ROOT) + logWarning(s"Encounter a failure during looking up function:" + + s" ${SparkUtils.exceptionString(error)}") if (!hiveFunctions.contains(functionName)) { - failFunctionLookup(funcName) + failFunctionLookup(funcName, Some(SparkUtils.exceptionString(error))) } // TODO: Remove this fallback path once we implement the list of fallback functions @@ -150,12 +153,12 @@ private[sql] class HiveSessionCatalog( val functionInfo = { try { Option(HiveFunctionRegistry.getFunctionInfo(functionName)).getOrElse( - failFunctionLookup(funcName)) + failFunctionLookup(funcName, Some(SparkUtils.exceptionString(error)))) } catch { // If HiveFunctionRegistry.getFunctionInfo throws an exception, // we are failing to load a Hive builtin function, which means that // the given function is not a Hive builtin function. - case NonFatal(e) => failFunctionLookup(funcName) + case NonFatal(e) => failFunctionLookup(funcName, Some(SparkUtils.exceptionString(e))) } } val className = functionInfo.getFunctionClass.getName From e1866d104edf42d14f1f34c537031b02ae6f9d8a Mon Sep 17 00:00:00 2001 From: zhoukang Date: Wed, 19 Dec 2018 19:31:39 +0800 Subject: [PATCH 02/11] Refine comment --- .../sql/catalyst/analysis/NoSuchItemException.scala | 10 ++++++---- .../spark/sql/catalyst/catalog/SessionCatalog.scala | 4 ++-- .../org/apache/spark/sql/hive/HiveSessionCatalog.scala | 10 +++++----- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala index 3866dedceed40..a32cbd451211b 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala @@ -19,6 +19,7 @@ package org.apache.spark.sql.catalyst.analysis import org.apache.spark.sql.AnalysisException import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec +import org.apache.spark.util.Utils /** @@ -40,11 +41,12 @@ class NoSuchPartitionException( class NoSuchPermanentFunctionException(db: String, func: String) extends AnalysisException(s"Function '$func' not found in database '$db'") -class NoSuchFunctionException(db: String, func: String, rootCause: Option[String] = None) +class NoSuchFunctionException(db: String, func: String, cause: Option[Throwable] = None) extends AnalysisException( - s"Undefined function: '$func'. This function is neither a registered temporary function nor " + - s"a permanent function registered in the database '$db'." + - s"Exception thrown during look up: ${rootCause.getOrElse("None")}") + s"Undefined function: '$func'. This function is neither a registered temporary function nor " + + s"a permanent function registered in the database '$db'." + + s"${cause.map(th => s"Exception thrown during look up:" + + s" ${Utils.exceptionString(th)}").getOrElse("")}") class NoSuchPartitionsException(db: String, table: String, specs: Seq[TablePartitionSpec]) extends AnalysisException( 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 f73b5ba471e83..1ebb37ef369c3 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 @@ -1210,9 +1210,9 @@ class SessionCatalog( } protected def failFunctionLookup( - name: FunctionIdentifier, rootCause: Option[String] = None): Nothing = { + name: FunctionIdentifier, cause: Option[Throwable] = None): Nothing = { throw new NoSuchFunctionException( - db = name.database.getOrElse(getCurrentDatabase), func = name.funcName, rootCause) + db = name.database.getOrElse(getCurrentDatabase), func = name.funcName, cause) } /** diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala index 6d46bf490a33b..c00c514348487 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala @@ -36,7 +36,7 @@ import org.apache.spark.sql.catalyst.parser.ParserInterface import org.apache.spark.sql.hive.HiveShim.HiveFunctionWrapper import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.types.{DecimalType, DoubleType} -import org.apache.spark.util.{Utils => SparkUtils} +import org.apache.spark.util.Utils private[sql] class HiveSessionCatalog( @@ -143,9 +143,9 @@ private[sql] class HiveSessionCatalog( // Hive is case insensitive. val functionName = funcName.unquotedString.toLowerCase(Locale.ROOT) logWarning(s"Encounter a failure during looking up function:" + - s" ${SparkUtils.exceptionString(error)}") + s" ${Utils.exceptionString(error)}") if (!hiveFunctions.contains(functionName)) { - failFunctionLookup(funcName, Some(SparkUtils.exceptionString(error))) + failFunctionLookup(funcName, Some(error)) } // TODO: Remove this fallback path once we implement the list of fallback functions @@ -153,12 +153,12 @@ private[sql] class HiveSessionCatalog( val functionInfo = { try { Option(HiveFunctionRegistry.getFunctionInfo(functionName)).getOrElse( - failFunctionLookup(funcName, Some(SparkUtils.exceptionString(error)))) + failFunctionLookup(funcName, Some(error))) } catch { // If HiveFunctionRegistry.getFunctionInfo throws an exception, // we are failing to load a Hive builtin function, which means that // the given function is not a Hive builtin function. - case NonFatal(e) => failFunctionLookup(funcName, Some(SparkUtils.exceptionString(e))) + case NonFatal(e) => failFunctionLookup(funcName, Some(e)) } } val className = functionInfo.getFunctionClass.getName From ec208e02cd90a5065386510ad3f60c84d6918533 Mon Sep 17 00:00:00 2001 From: zhoukang Date: Thu, 27 Dec 2018 19:30:57 +0800 Subject: [PATCH 03/11] Add unit test --- .../spark/sql/catalyst/catalog/SessionCatalog.scala | 2 +- .../spark/sql/catalyst/catalog/SessionCatalogSuite.scala | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) 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 1ebb37ef369c3..a54e5e274e1d8 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 @@ -1209,7 +1209,7 @@ class SessionCatalog( databaseExists(db) && externalCatalog.functionExists(db, name.funcName) } - protected def failFunctionLookup( + protected[sql] def failFunctionLookup( name: FunctionIdentifier, cause: Option[Throwable] = None): Nothing = { throw new NoSuchFunctionException( db = name.database.getOrElse(getCurrentDatabase), func = name.funcName, cause) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala index 50496a0410528..eee1cb8cf9e39 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala @@ -1445,4 +1445,12 @@ abstract class SessionCatalogSuite extends AnalysisTest { } } } + + test("SPARK-24544: test print actual failure cause when look up function failed") { + withBasicCatalog { catalog => + intercept[NoSuchFunctionException] { + catalog.failFunctionLookup(FunctionIdentifier("failureFunc"), Some(new Exception("Test"))) + } + } + } } From 26aeb5806a3bee978707feb78755a43cba02ba85 Mon Sep 17 00:00:00 2001 From: zhoukang Date: Sat, 29 Dec 2018 12:33:43 +0800 Subject: [PATCH 04/11] Refine comment --- .../spark/sql/catalyst/analysis/NoSuchItemException.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala index a32cbd451211b..5866031c31454 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala @@ -44,9 +44,7 @@ class NoSuchPermanentFunctionException(db: String, func: String) class NoSuchFunctionException(db: String, func: String, cause: Option[Throwable] = None) extends AnalysisException( s"Undefined function: '$func'. This function is neither a registered temporary function nor " + - s"a permanent function registered in the database '$db'." + - s"${cause.map(th => s"Exception thrown during look up:" + - s" ${Utils.exceptionString(th)}").getOrElse("")}") + s"a permanent function registered in the database '$db'.", cause = cause) class NoSuchPartitionsException(db: String, table: String, specs: Seq[TablePartitionSpec]) extends AnalysisException( From 95ba6e7cced0010fd10934c28d216560d17be6da Mon Sep 17 00:00:00 2001 From: zhoukang Date: Sat, 29 Dec 2018 13:05:40 +0800 Subject: [PATCH 05/11] Update style --- .../spark/sql/catalyst/analysis/NoSuchItemException.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala index 5866031c31454..a5cd407c6f417 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala @@ -43,8 +43,8 @@ class NoSuchPermanentFunctionException(db: String, func: String) class NoSuchFunctionException(db: String, func: String, cause: Option[Throwable] = None) extends AnalysisException( - s"Undefined function: '$func'. This function is neither a registered temporary function nor " + - s"a permanent function registered in the database '$db'.", cause = cause) + s"Undefined function: '$func'. This function is neither a registered temporary function nor " + + s"a permanent function registered in the database '$db'.", cause = cause) class NoSuchPartitionsException(db: String, table: String, specs: Seq[TablePartitionSpec]) extends AnalysisException( From 1bff63cadae7981b8f9e4633cfa3a3501cd4cb90 Mon Sep 17 00:00:00 2001 From: zhoukang Date: Sat, 29 Dec 2018 16:44:24 +0800 Subject: [PATCH 06/11] Remove useless import --- .../apache/spark/sql/catalyst/analysis/NoSuchItemException.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala index a5cd407c6f417..8bf6f69f3b17a 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/NoSuchItemException.scala @@ -19,7 +19,6 @@ package org.apache.spark.sql.catalyst.analysis import org.apache.spark.sql.AnalysisException import org.apache.spark.sql.catalyst.catalog.CatalogTypes.TablePartitionSpec -import org.apache.spark.util.Utils /** From 820faf2c6e9050657c85fe1773107806f5a1d227 Mon Sep 17 00:00:00 2001 From: zhoukang Date: Sun, 30 Dec 2018 20:47:48 +0800 Subject: [PATCH 07/11] Refine comment --- .../scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala index c00c514348487..a534fc4b9c14e 100644 --- a/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala +++ b/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveSessionCatalog.scala @@ -142,7 +142,7 @@ private[sql] class HiveSessionCatalog( // built-in function. // Hive is case insensitive. val functionName = funcName.unquotedString.toLowerCase(Locale.ROOT) - logWarning(s"Encounter a failure during looking up function:" + + logWarning("Encountered a failure during looking up function:" + s" ${Utils.exceptionString(error)}") if (!hiveFunctions.contains(functionName)) { failFunctionLookup(funcName, Some(error)) From 240f3ab24b7677b482843dd25ac62274e910a48b Mon Sep 17 00:00:00 2001 From: zhoukang Date: Tue, 1 Jan 2019 08:46:30 +0800 Subject: [PATCH 08/11] Refine --- .../spark/sql/catalyst/catalog/SessionCatalogSuite.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala index eee1cb8cf9e39..9cbd55dd1d679 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala @@ -1448,9 +1448,11 @@ abstract class SessionCatalogSuite extends AnalysisTest { test("SPARK-24544: test print actual failure cause when look up function failed") { withBasicCatalog { catalog => - intercept[NoSuchFunctionException] { - catalog.failFunctionLookup(FunctionIdentifier("failureFunc"), Some(new Exception("Test"))) + val cause = intercept[NoSuchFunctionException] { + catalog.failFunctionLookup(FunctionIdentifier("failureFunc"), Some(new Exception("Actual error!"))) } + + assert(cause.getMessage.contains("Actual error!")) } } } From 6d3bd190e6301cc2892524a0884e5eddd94e306a Mon Sep 17 00:00:00 2001 From: zhoukang Date: Tue, 1 Jan 2019 08:47:32 +0800 Subject: [PATCH 09/11] Refine --- .../spark/sql/catalyst/catalog/SessionCatalogSuite.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala index 9cbd55dd1d679..c152b6c7f1dc6 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala @@ -1449,10 +1449,11 @@ abstract class SessionCatalogSuite extends AnalysisTest { test("SPARK-24544: test print actual failure cause when look up function failed") { withBasicCatalog { catalog => val cause = intercept[NoSuchFunctionException] { - catalog.failFunctionLookup(FunctionIdentifier("failureFunc"), Some(new Exception("Actual error!"))) + catalog.failFunctionLookup(FunctionIdentifier("failureFunc"), + Some(new Exception("Actual error"))) } - assert(cause.getMessage.contains("Actual error!")) + assert(cause.getMessage.contains("Actual error")) } } } From 97493c95e3068afe17b4665a773e8fadcd459ed3 Mon Sep 17 00:00:00 2001 From: zhoukang Date: Tue, 1 Jan 2019 09:57:09 +0800 Subject: [PATCH 10/11] Refine --- .../apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala index c152b6c7f1dc6..f262490b1c8c7 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala @@ -1453,7 +1453,7 @@ abstract class SessionCatalogSuite extends AnalysisTest { Some(new Exception("Actual error"))) } - assert(cause.getMessage.contains("Actual error")) + assert(cause.cause.get.getMessage.contains("Actual error")) } } } From d1d4adfe0620253f65e5c54366768c695823ae28 Mon Sep 17 00:00:00 2001 From: zhoukang Date: Tue, 1 Jan 2019 10:00:01 +0800 Subject: [PATCH 11/11] Add comment --- .../spark/sql/catalyst/catalog/SessionCatalogSuite.scala | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala index f262490b1c8c7..88cad8436416b 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalogSuite.scala @@ -1453,6 +1453,9 @@ abstract class SessionCatalogSuite extends AnalysisTest { Some(new Exception("Actual error"))) } + // fullStackTrace will be printed, but `cause.getMessage` has been + // override in `AnalysisException`,so here we get the root cause + // exception message for check. assert(cause.cause.get.getMessage.contains("Actual error")) } }