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 2022/11/20 13:56:58 UTC

[GitHub] [spark] grundprinzip opened a new pull request, #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

grundprinzip opened a new pull request, #38728:
URL: https://github.com/apache/spark/pull/38728

   ### What changes were proposed in this pull request?
   Migrate existing custom exceptions in Spark Connect to use the proper Spark exceptions.
   
   ### Why are the changes needed?
   Consistency
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   ### How was this patch tested?
   existing UT
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] itholic commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1027408367


##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -45,12 +46,18 @@ import org.apache.spark.util.Utils
 final case class InvalidPlanInput(
     private val message: String = "",
     private val cause: Throwable = None.orNull)
-    extends Exception(message, cause)
+    extends SparkException(

Review Comment:
   Can we have test for the exception ??
   
   If it's already exists, let's convert the existing test to use `checkError` to check error class and parameters generated properly.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] HyukjinKwon commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1027385407


##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -45,12 +46,18 @@ import org.apache.spark.util.Utils
 final case class InvalidPlanInput(
     private val message: String = "",
     private val cause: Throwable = None.orNull)
-    extends Exception(message, cause)
+    extends SparkException(

Review Comment:
   cc @MaxGekk @itholic HELP!!



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] github-actions[bot] closed pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions
URL: https://github.com/apache/spark/pull/38728


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033662980


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -134,6 +134,97 @@
         "message" : [
           "Error instantiating GRPC interceptor: <msg>"
         ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>"
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION": {

Review Comment:
   I regenerated the json file based on the SparkThrowableSuite.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033016586


##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -45,12 +46,18 @@ import org.apache.spark.util.Utils
 final case class InvalidPlanInput(
     private val message: String = "",
     private val cause: Throwable = None.orNull)
-    extends Exception(message, cause)
+    extends SparkException(
+      errorClass = "CONNECT.INVALID_PLAN_INPUT",
+      messageParameters = Map("msg" -> message),
+      cause = cause)
 
 final case class InvalidCommandInput(
     private val message: String = "",
     private val cause: Throwable = null)
-    extends Exception(message, cause)
+    extends SparkException(

Review Comment:
   Done



##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -45,12 +46,18 @@ import org.apache.spark.util.Utils
 final case class InvalidPlanInput(
     private val message: String = "",
     private val cause: Throwable = None.orNull)
-    extends Exception(message, cause)
+    extends SparkException(

Review Comment:
   done



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on PR #38728:
URL: https://github.com/apache/spark/pull/38728#issuecomment-1328349502

   @itholic Finally got around addressing your comments, please have another look. Thanks!


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1034083957


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {
+        "message" : [
+          "Values must contains at least 1 item."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE" : {
+        "message" : [
+          "When values contains more than 1 items, values and cols should have the same length."
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION" : {
+        "message" : [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_DATA_SOURCE" : {
+        "message" : [
+          "Unsupported input data source: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_EXPRESSION_TYPE" : {
+        "message" : [
+          "Unsupported expression type: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_JOIN_TYPE" : {
+        "message" : [
+          "Unsupported Join type: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_LITERAL_TYPE" : {
+        "message" : [
+          "Unsupported Literal type: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_SET_OPERATION" : {
+        "message" : [
+          "Unsupported Set operation: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_TYPE_CONVERSION" : {

Review Comment:
   Done



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {
+        "message" : [
+          "Values must contains at least 1 item."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE" : {
+        "message" : [
+          "When values contains more than 1 items, values and cols should have the same length."
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION" : {
+        "message" : [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_DATA_SOURCE" : {
+        "message" : [
+          "Unsupported input data source: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_EXPRESSION_TYPE" : {

Review Comment:
   Done



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {
+        "message" : [
+          "Values must contains at least 1 item."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE" : {
+        "message" : [
+          "When values contains more than 1 items, values and cols should have the same length."
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION" : {
+        "message" : [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_DATA_SOURCE" : {
+        "message" : [
+          "Unsupported input data source: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_EXPRESSION_TYPE" : {
+        "message" : [
+          "Unsupported expression type: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_JOIN_TYPE" : {
+        "message" : [
+          "Unsupported Join type: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_LITERAL_TYPE" : {

Review Comment:
   Done



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033266116


##########
connector/connect/src/test/scala/org/apache/spark/sql/connect/planner/SparkConnectPlannerSuite.scala:
##########
@@ -329,19 +346,25 @@ class SparkConnectPlannerSuite extends SparkFunSuite with SparkConnectPlanTest {
       .setAllColumnsAsKeys(true)
       .addColumnNames("test")
 
-    val e = intercept[InvalidPlanInput] {
-      transform(proto.Relation.newBuilder.setDeduplicate(deduplicate).build())
-    }
-    assert(
-      e.getMessage.contains("Cannot deduplicate on both all columns and a subset of columns"))
+    checkError(
+      exception = intercept[InvalidPlanInput] {
+        transform(proto.Relation.newBuilder.setDeduplicate(deduplicate).build())
+      },
+      errorClass = "CONNECT.INVALID_PLAN_INPUT",
+      parameters = Map("msg" -> "Cannot deduplicate on both all columns and a subset of columns"))

Review Comment:
   Happy to move the literals out. 



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] srielau commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
srielau commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1036118316


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {

Review Comment:
   Uhm, I'm now no smarter. The real question is: Will users know what to do with this error?



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1034987395


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {

Review Comment:
   It's the underlying operator.
   
   ```
    def fillna(
           self,
           value: Union["LiteralType", Dict[str, "LiteralType"]],
           subset: Optional[Union[str, Tuple[str, ...], List[str]]] = None,
       ) -> "DataFrame":
           """Replace null values, alias for ``na.fill()``.
           :func:`DataFrame.fillna` and :func:`DataFrameNaFunctions.fill` are aliases of each other.
   
   ```



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033663506


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -134,6 +134,97 @@
         "message" : [
           "Error instantiating GRPC interceptor: <msg>"
         ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>"
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION": {
+        "message": [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY": {
+        "message": [
+          "Values must contains at least 1 item"

Review Comment:
   Done.



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -134,6 +134,97 @@
         "message" : [
           "Error instantiating GRPC interceptor: <msg>"
         ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>"
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION": {
+        "message": [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY": {
+        "message": [
+          "Values must contains at least 1 item"
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE": {
+        "message": [
+          "When values contains more than 1 items, ",
+          "values and cols should have the same length!"

Review Comment:
   Done.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033662258


##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -265,7 +280,10 @@ class SparkConnectPlanner(session: SparkSession) {
         // so we call filter instead of find.
         val cols = allColumns.filter(col => resolver(col.name, colName))
         if (cols.isEmpty) {
-          throw InvalidPlanInput(s"Invalid deduplicate column ${colName}")
+          throw new SparkException(
+            errorClass = "CONNECT.INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN",
+            messageParameters = Map("col" -> colName),

Review Comment:
   Done



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on PR #38728:
URL: https://github.com/apache/spark/pull/38728#issuecomment-1329372072

   I will add tests for the missing cases.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] srielau commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
srielau commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1034330247


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {

Review Comment:
   What is NA?



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] AmplabJenkins commented on pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #38728:
URL: https://github.com/apache/spark/pull/38728#issuecomment-1321155240

   Can one of the admins verify this patch?


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] github-actions[bot] commented on pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38728:
URL: https://github.com/apache/spark/pull/38728#issuecomment-1464712334

   We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
   If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] srielau commented on pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
srielau commented on PR #38728:
URL: https://github.com/apache/spark/pull/38728#issuecomment-1330129095

   General comment. You use CONNECT as error class, everything else is a sub error class. Many of these are INVALID_PLAN.
   How many errors total do you expect?
   Note that at present we support only two levels.
   Would it make sense to use CONNECT (or an abbreviation of it) as a prefix and then perhaps subclass at INVALID_PLAN:
   CONNECT_INVALID_PLAN.xxx
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] MaxGekk commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033537819


##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -504,7 +545,11 @@ class SparkConnectPlanner(session: SparkSession) {
       case proto.Join.JoinType.JOIN_TYPE_LEFT_OUTER => LeftOuter
       case proto.Join.JoinType.JOIN_TYPE_RIGHT_OUTER => RightOuter
       case proto.Join.JoinType.JOIN_TYPE_LEFT_SEMI => LeftSemi
-      case _ => throw InvalidPlanInput(s"Join type ${t} is not supported")
+      case _ =>
+        throw new SparkException(
+          errorClass = "CONNECT.INVALID_PLAN_UNSUPPORTED_JOIN_TYPE",
+          messageParameters = Map("type" -> s"${t}"),

Review Comment:
   nit:
   ```suggestion
             messageParameters = Map("type" -> s"$t"),
   ```



##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -265,7 +280,10 @@ class SparkConnectPlanner(session: SparkSession) {
         // so we call filter instead of find.
         val cols = allColumns.filter(col => resolver(col.name, colName))
         if (cols.isEmpty) {
-          throw InvalidPlanInput(s"Invalid deduplicate column ${colName}")
+          throw new SparkException(
+            errorClass = "CONNECT.INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN",
+            messageParameters = Map("col" -> colName),

Review Comment:
   Could you quote the column by `toSQLId()`, please.



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -134,6 +134,97 @@
         "message" : [
           "Error instantiating GRPC interceptor: <msg>"
         ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>"
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION": {
+        "message": [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY": {
+        "message": [
+          "Values must contains at least 1 item"
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE": {
+        "message": [
+          "When values contains more than 1 items, ",
+          "values and cols should have the same length!"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_LITERAL_TYPE": {
+        "message": [
+          "Unsupported Literal type: <type>"
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message": [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message": [
+          "Deduplicate must be called with either all or a subset of coulumn names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message": [
+          "Invalid column name used for Deduplicate: <col>"
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message": [
+          "Read from DataSource requires a format parameter"

Review Comment:
   ```suggestion
             "Read from DataSource requires a format parameter."
   ```



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -134,6 +134,97 @@
         "message" : [
           "Error instantiating GRPC interceptor: <msg>"
         ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>"
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION": {
+        "message": [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY": {
+        "message": [
+          "Values must contains at least 1 item"
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE": {
+        "message": [
+          "When values contains more than 1 items, ",
+          "values and cols should have the same length!"

Review Comment:
   ```suggestion
             "When values contains more than 1 items, values and cols should have the same length."
   ```



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -134,6 +134,97 @@
         "message" : [
           "Error instantiating GRPC interceptor: <msg>"
         ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>"
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION": {
+        "message": [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY": {
+        "message": [
+          "Values must contains at least 1 item"

Review Comment:
   ```suggestion
             "Values must contains at least 1 item."
   ```



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -134,6 +134,97 @@
         "message" : [
           "Error instantiating GRPC interceptor: <msg>"
         ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>"
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION": {

Review Comment:
   Wrong the formatting here and bellow:
   ```suggestion
         "INVALID_PLAN_UNKNOWN_RELATION" : {
   ```



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -134,6 +134,97 @@
         "message" : [
           "Error instantiating GRPC interceptor: <msg>"
         ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>"
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION": {
+        "message": [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY": {
+        "message": [
+          "Values must contains at least 1 item"
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE": {
+        "message": [
+          "When values contains more than 1 items, ",
+          "values and cols should have the same length!"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_LITERAL_TYPE": {
+        "message": [
+          "Unsupported Literal type: <type>"
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message": [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message": [
+          "Deduplicate must be called with either all or a subset of coulumn names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message": [
+          "Invalid column name used for Deduplicate: <col>"
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message": [
+          "Read from DataSource requires a format parameter"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_DATA_SOURCE" : {
+        "message": [
+          "Unsupported input data source: <type>"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_EXPRESSION_TYPE" : {
+        "message": [
+          "Unsupported expression type: <type>"
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message": [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message": [
+          "Invalid Set operation: <type> does not support <param>"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_SET_OPERATION" : {
+        "message": [
+          "Unsupported Set operation: <type>"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_JOIN_TYPE" : {
+        "message": [
+          "Unsupported Join type: <type>"
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message": [
+          "Using columns or join conditions cannot be set at the same time in Join"

Review Comment:
   ```suggestion
             "Using columns or join conditions cannot be set at the same time in Join."
   ```



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] MaxGekk commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033737542


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {

Review Comment:
   Is it tested somewhere? If not, could add a test, please.



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {
+        "message" : [
+          "Values must contains at least 1 item."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE" : {

Review Comment:
   Is there a test for the error?



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {
+        "message" : [
+          "Values must contains at least 1 item."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE" : {
+        "message" : [
+          "When values contains more than 1 items, values and cols should have the same length."
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION" : {
+        "message" : [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_DATA_SOURCE" : {
+        "message" : [
+          "Unsupported input data source: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_EXPRESSION_TYPE" : {

Review Comment:
   Could you add a test for this, please



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {
+        "message" : [
+          "Values must contains at least 1 item."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE" : {
+        "message" : [
+          "When values contains more than 1 items, values and cols should have the same length."
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION" : {
+        "message" : [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_DATA_SOURCE" : {
+        "message" : [
+          "Unsupported input data source: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_EXPRESSION_TYPE" : {
+        "message" : [
+          "Unsupported expression type: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_JOIN_TYPE" : {
+        "message" : [
+          "Unsupported Join type: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_LITERAL_TYPE" : {
+        "message" : [
+          "Unsupported Literal type: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_SET_OPERATION" : {
+        "message" : [
+          "Unsupported Set operation: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_TYPE_CONVERSION" : {

Review Comment:
   Check this error class, please.



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {
+        "message" : [
+          "Values must contains at least 1 item."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE" : {
+        "message" : [
+          "When values contains more than 1 items, values and cols should have the same length."
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION" : {
+        "message" : [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_DATA_SOURCE" : {
+        "message" : [
+          "Unsupported input data source: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_EXPRESSION_TYPE" : {
+        "message" : [
+          "Unsupported expression type: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_JOIN_TYPE" : {
+        "message" : [
+          "Unsupported Join type: <type>."
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_LITERAL_TYPE" : {

Review Comment:
   Please, add a test for this error class.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1034062284


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {

Review Comment:
   Done



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {
+        "message" : [
+          "Values must contains at least 1 item."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE" : {

Review Comment:
   Done



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] itholic commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
itholic commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1027408511


##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -45,12 +46,18 @@ import org.apache.spark.util.Utils
 final case class InvalidPlanInput(
     private val message: String = "",
     private val cause: Throwable = None.orNull)
-    extends Exception(message, cause)
+    extends SparkException(
+      errorClass = "CONNECT.INVALID_PLAN_INPUT",
+      messageParameters = Map("msg" -> message),
+      cause = cause)
 
 final case class InvalidCommandInput(
     private val message: String = "",
     private val cause: Throwable = null)
-    extends Exception(message, cause)
+    extends SparkException(

Review Comment:
   ditto



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033665152


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -134,6 +134,97 @@
         "message" : [
           "Error instantiating GRPC interceptor: <msg>"
         ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>"
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION": {
+        "message": [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY": {
+        "message": [
+          "Values must contains at least 1 item"
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE": {
+        "message": [
+          "When values contains more than 1 items, ",
+          "values and cols should have the same length!"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_LITERAL_TYPE": {
+        "message": [
+          "Unsupported Literal type: <type>"
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message": [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message": [
+          "Deduplicate must be called with either all or a subset of coulumn names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message": [
+          "Invalid column name used for Deduplicate: <col>"
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message": [
+          "Read from DataSource requires a format parameter"

Review Comment:
   Done.



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -134,6 +134,97 @@
         "message" : [
           "Error instantiating GRPC interceptor: <msg>"
         ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>"
+        ]
+      },
+      "INVALID_PLAN_UNKNOWN_RELATION": {
+        "message": [
+          "Relation: <param> not supported."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY": {
+        "message": [
+          "Values must contains at least 1 item"
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_SIZE": {
+        "message": [
+          "When values contains more than 1 items, ",
+          "values and cols should have the same length!"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_LITERAL_TYPE": {
+        "message": [
+          "Unsupported Literal type: <type>"
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message": [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message": [
+          "Deduplicate must be called with either all or a subset of coulumn names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message": [
+          "Invalid column name used for Deduplicate: <col>"
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message": [
+          "Read from DataSource requires a format parameter"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_DATA_SOURCE" : {
+        "message": [
+          "Unsupported input data source: <type>"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_EXPRESSION_TYPE" : {
+        "message": [
+          "Unsupported expression type: <type>"
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message": [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message": [
+          "Invalid Set operation: <type> does not support <param>"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_SET_OPERATION" : {
+        "message": [
+          "Unsupported Set operation: <type>"
+        ]
+      },
+      "INVALID_PLAN_UNSUPPORTED_JOIN_TYPE" : {
+        "message": [
+          "Unsupported Join type: <type>"
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message": [
+          "Using columns or join conditions cannot be set at the same time in Join"

Review Comment:
   Done.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033662577


##########
connector/connect/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -504,7 +545,11 @@ class SparkConnectPlanner(session: SparkSession) {
       case proto.Join.JoinType.JOIN_TYPE_LEFT_OUTER => LeftOuter
       case proto.Join.JoinType.JOIN_TYPE_RIGHT_OUTER => RightOuter
       case proto.Join.JoinType.JOIN_TYPE_LEFT_SEMI => LeftSemi
-      case _ => throw InvalidPlanInput(s"Join type ${t} is not supported")
+      case _ =>
+        throw new SparkException(
+          errorClass = "CONNECT.INVALID_PLAN_UNSUPPORTED_JOIN_TYPE",
+          messageParameters = Map("type" -> s"${t}"),

Review Comment:
   Done



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] grundprinzip commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
grundprinzip commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033745251


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -132,7 +132,97 @@
       },
       "INTERCEPTOR_RUNTIME_ERROR" : {
         "message" : [
-          "Error instantiating GRPC interceptor: <msg>"
+          "Error instantiating GRPC interceptor: <msg>."
+        ]
+      },
+      "INVALID_COMMAND_WRITE_INVALID_BUCKET_BY" : {
+        "message" : [
+          "BucketBy must specify a bucket count > 0, received <bucket> instead."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT" : {
+        "message" : [
+          "Deduplicate must be called with either all or a subset of column names to deduplicate."
+        ]
+      },
+      "INVALID_PLAN_DEDUPLICATE_INVALID_INPUT_COLUMN" : {
+        "message" : [
+          "Invalid column name used for Deduplicate: <col>."
+        ]
+      },
+      "INVALID_PLAN_EMPTY_INPUT" : {
+        "message" : [
+          "Input relation must not be empty."
+        ]
+      },
+      "INVALID_PLAN_INPUT" : {
+        "message" : [
+          "Error transforming Spark Connect plan: <msg>."
+        ]
+      },
+      "INVALID_PLAN_INVALID_ALIAS" : {
+        "message" : [
+          "Alias expressions with more than 1 identifier must not use optional metadata."
+        ]
+      },
+      "INVALID_PLAN_INVALID_DATA_SOURCE" : {
+        "message" : [
+          "Read from DataSource requires a format parameter."
+        ]
+      },
+      "INVALID_PLAN_INVALID_JOIN_CONDITION" : {
+        "message" : [
+          "Using columns or join conditions cannot be set at the same time in Join."
+        ]
+      },
+      "INVALID_PLAN_INVALID_SET_OPERATION" : {
+        "message" : [
+          "Invalid Set operation: <type> does not support <param>."
+        ]
+      },
+      "INVALID_PLAN_TRANSFORM_NA_FILL_EMPTY" : {

Review Comment:
   Yes, there tests for all of these messages. Please see the SparkConnectPlannerSuite and SparkConnectProtoSuite.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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


[GitHub] [spark] MaxGekk commented on a diff in pull request #38728: [SPARK-41204] [CONNECT] Migrate custom exceptions to use Spark exceptions

Posted by GitBox <gi...@apache.org>.
MaxGekk commented on code in PR #38728:
URL: https://github.com/apache/spark/pull/38728#discussion_r1033260567


##########
connector/connect/src/test/scala/org/apache/spark/sql/connect/planner/SparkConnectPlannerSuite.scala:
##########
@@ -329,19 +346,25 @@ class SparkConnectPlannerSuite extends SparkFunSuite with SparkConnectPlanTest {
       .setAllColumnsAsKeys(true)
       .addColumnNames("test")
 
-    val e = intercept[InvalidPlanInput] {
-      transform(proto.Relation.newBuilder.setDeduplicate(deduplicate).build())
-    }
-    assert(
-      e.getMessage.contains("Cannot deduplicate on both all columns and a subset of columns"))
+    checkError(
+      exception = intercept[InvalidPlanInput] {
+        transform(proto.Relation.newBuilder.setDeduplicate(deduplicate).build())
+      },
+      errorClass = "CONNECT.INVALID_PLAN_INPUT",
+      parameters = Map("msg" -> "Cannot deduplicate on both all columns and a subset of columns"))

Review Comment:
   Would it possible to move all the plain text to `error-classes.json`. Could you image that we will translate errors to local languages in the future, or tech editors will fix some typos in the texts. In your case, we will have to rebuild Spark which is not nice approach.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

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