You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "amousavigourabi (via GitHub)" <gi...@apache.org> on 2023/04/25 21:58:10 UTC

[GitHub] [spark] amousavigourabi opened a new pull request, #40951: [SPARK-43250] Replace the error class `_LEGACY_ERROR_TEMP_2014` with an internal error

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

   ### What changes were proposed in this pull request?
   In this PR I propose to replace the legacy error class `_LEGACY_ERROR_TEMP_2014` with an internal error as it is not triggered by the user space.
   
   ### Why are the changes needed?
   As the error is not triggered by the user space, the legacy error class can be replaced by an internal error.
   
   ### Does this PR introduce _any_ user-facing change?
   No.
   
   ### How was this patch tested?
   No new tests were added and no tests needed changing because of the nature of the updated 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] MaxGekk closed pull request #40951: [SPARK-43250][SQL] Replace the error class `_LEGACY_ERROR_TEMP_2014` with an internal error

Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk closed pull request #40951: [SPARK-43250][SQL] Replace the error class `_LEGACY_ERROR_TEMP_2014` with an internal error
URL: https://github.com/apache/spark/pull/40951


-- 
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] amousavigourabi commented on pull request #40951: [SPARK-43250][SQL] Replace the error class `_LEGACY_ERROR_TEMP_2014` with an internal error

Posted by "amousavigourabi (via GitHub)" <gi...@apache.org>.
amousavigourabi commented on PR #40951:
URL: https://github.com/apache/spark/pull/40951#issuecomment-1525810649

   @MaxGekk could you help review this?


-- 
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 pull request #40951: [SPARK-43250][SQL] Replace the error class `_LEGACY_ERROR_TEMP_2014` with an internal error

Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #40951:
URL: https://github.com/apache/spark/pull/40951#issuecomment-1533408418

   +1, LGTM. Merging to master.
   Thank you, @amousavigourabi.


-- 
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 #40951: [SPARK-43250][SQL] Replace the error class `_LEGACY_ERROR_TEMP_2014` with an internal error

Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #40951:
URL: https://github.com/apache/spark/pull/40951#discussion_r1181482386


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala:
##########
@@ -413,10 +413,10 @@ private[sql] object QueryExecutionErrors extends QueryErrorsBase {
       messageParameters = Map("frequencyExpression" -> frequencyExpression.sql))
   }
 
-  def addNewFunctionMismatchedWithFunctionError(funcName: String): SparkIllegalArgumentException = {
-    new SparkIllegalArgumentException(
-      errorClass = "_LEGACY_ERROR_TEMP_2014",
-      messageParameters = Map("funcName" -> funcName))
+  def addNewFunctionMismatchedWithFunctionError(funcName: String): Throwable = {
+    SparkException.internalError(
+      "Cannot add new function to generated class, " +
+        s"failed to match $funcName at addNewFunction")

Review Comment:
   Could you quote the names as we do in other places, please:
   ```suggestion
           s"failed to match ${toSQLId(funcName)} at `addNewFunction`.")
   ```



-- 
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 pull request #40951: [SPARK-43250][SQL] Replace the error class `_LEGACY_ERROR_TEMP_2014` with an internal error

Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #40951:
URL: https://github.com/apache/spark/pull/40951#issuecomment-1533415423

   @amousavigourabi Congratulations with your first contribution to Apache Spark!


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