You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/02/01 23:43:25 UTC

[GitHub] [spark] allisonwang-db commented on a change in pull request #31316: [SPARK-33599][SQL][FOLLOWUP] Group exception messages in catalyst/analysis

allisonwang-db commented on a change in pull request #31316:
URL: https://github.com/apache/spark/pull/31316#discussion_r568214737



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
##########
@@ -732,4 +732,20 @@ private[spark] object QueryCompilationErrors {
     new InvalidUDFClassException(s"No handler for UDAF '$name'. " +
       "Use sparkSession.udf.register(...) instead.")
   }
+
+   def unexpectedColumnExpressionError(column: Expression): Throwable = {

Review comment:
       nit: extra space before def

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala
##########
@@ -332,4 +332,9 @@ object QueryExecutionErrors {
   def compilerError(e: CompileException): Throwable = {
     new CompileException(failedToCompileMsg(e), e.getLocation)
   }
+
+  def invalidPartitionExpressionsError(sortOrders: Seq[Any]): Throwable = {

Review comment:
       invalidRepartitionExpressionsError

##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
##########
@@ -732,4 +732,20 @@ private[spark] object QueryCompilationErrors {
     new InvalidUDFClassException(s"No handler for UDAF '$name'. " +
       "Use sparkSession.udf.register(...) instead.")
   }
+
+   def unexpectedColumnExpressionError(column: Expression): Throwable = {
+    new AnalysisException(s"[BUG] unexpected column expression: $column")
+  }
+
+  def databaseFromV1SessionCatalogNotSpecifiedError(): Throwable = {
+    new AnalysisException("Database from v1 session catalog is not specified")
+  }
+
+  def nestedDatabaseUnsupportedByV1SessionCatalogError(catalog: String): Throwable = {
+    new AnalysisException(s"Nested databases are not supported by v1 session catalog: $catalog")
+  }
+
+  def invalidCallFunctionOnUnresolvedObjectError(function: String): String = {

Review comment:
       For this one, I think it would be better to update all `new UnresolvedException` instead of adding the error message here. For example
   ```scala
   def invalidFunctionCallOnUnresolvedObjectError(function: String): Throwable = {
     new UnresolvedException(s"Invalid call to $function on unresolved object")
   }
   ```
   What do you think?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org