[SPARK-40663][SQL](Final) Migrate execution errors onto error classes #38177

itholic · 2022-10-09T08:52:39Z

What changes were proposed in this pull request?

This the final PR that proposes to migrate the execution errors onto temporary error classes with the prefix _LEGACY_ERROR_TEMP

The error classes are prefixed with _LEGACY_ERROR_TEMP_ indicates the dev-facing error messages, and won't be exposed to end users.

Why are the changes needed?

To speed-up the error class migration.

The migration on temporary error classes allow us to analyze the errors, so we can detect the most popular error classes.

Does this PR introduce any user-facing change?

No

How was this patch tested?

$ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite"
$ build/sbt "test:testOnly *SQLQuerySuite"
$ build/sbt -Phive-thriftserver "hive-thriftserver/testOnly org.apache.spark.sql.hive.thriftserver.ThriftServerQueryTestSuite"

…asses

itholic · 2022-10-18T13:47:24Z

This is ready for review, @MaxGekk FYI

MaxGekk · 2022-10-18T16:14:56Z

core/src/main/resources/error/error-classes.json

+  },
+  "_LEGACY_ERROR_TEMP_2277" : {
+    "message" : [
+      "Number of dynamic partitions created is <numWrittenParts>\", which is more than <maxDynamicPartitions>\". To solve this try to set <maxDynamicPartitionsKey> to at least <numWrittenParts>."


Could you remove " around \", which is more than <maxDynamicPartitions>\"

MaxGekk · 2022-10-18T16:15:44Z

core/src/main/resources/error/error-classes.json

+  },
+  "_LEGACY_ERROR_TEMP_2278" : {
+    "message" : [
+      "The input $valueType '<input>' does not match the given number format: '<format>'"


$valueType -> <valueType>

itholic · 2022-10-19T05:08:54Z

Thanks for the review, @MaxGekk !

Just adjusted the comments

…0540-2276-2300

MaxGekk · 2022-10-20T05:32:22Z

@itholic Could you re-trigger GAs and remove [WIP] if this PR is ready.

…0540-2276-2300

itholic · 2022-10-20T05:45:35Z

Thanks! Just re-triggered the GA and fix the title!

MaxGekk

Waiting for CI.

MaxGekk · 2022-10-21T04:51:43Z

@itholic Please, fix the test failure:

[info] - postgreSQL/numeric.sql *** FAILED *** (49 seconds, 862 milliseconds)
[info]   postgreSQL/numeric.sql
[info]   Expected "...g.apache.spark.Spark[IllegalArgumentException
[info]   {
[info]     "errorClass" : "_LEGACY_ERROR_TEMP_2278",
[info]     "messageParameters" : {
[info]       "format" : "99G999G999",
[info]       "input" : "-34,338,492]"
[info]     }
[info]   }", but got "...g.apache.spark.Spark[Exception
[info]   {
[info]     "errorClass" : "INTERNAL_ERROR",
[info]     "messageParameters" : {
[info]       "message" : "Undefined error message parameter for error class: '_LEGACY_ERROR_TEMP_2278'. Parameters: Map(input -> -34,338,492, format -> 99G999G999)]"

MaxGekk · 2022-10-24T04:44:52Z

ping @itholic

…0540-2276-2300

itholic · 2022-10-24T05:15:23Z

...talyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/StringExpressionsSuite.scala

+      checkExceptionInExpression[SparkException](
+        toNumberExpr, "[INTERNAL_ERROR] Undefined error message parameter")


Sorry for late responding, @MaxGekk .

I've been thinking about if there is better way to test expression with error class here.

Please let me know if there is better way to test this ? 🙏

See #38177 (review)

Seems like I should also fix the test & generated golden files.

Let me fix soon!

…0540-2276-2300

MaxGekk · 2022-10-24T16:12:55Z

sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala

+        "input" -> input,
+        "format" -> format))


Missed valueType, see the format of the error class _LEGACY_ERROR_TEMP_2278.

Oops, I missed it... Thanks for figuring out!

itholic · 2022-10-25T04:38:51Z

Fixed & re-gened the golden files!

…0540-2276-2300

MaxGekk · 2022-10-26T07:13:03Z

+1, LGTM. All GAs passed. Merging to master.
Thank you, @itholic.

### What changes were proposed in this pull request? This the final PR that proposes to migrate the execution errors onto temporary error classes with the prefix `_LEGACY_ERROR_TEMP` The error classes are prefixed with `_LEGACY_ERROR_TEMP_` indicates the dev-facing error messages, and won't be exposed to end users. ### Why are the changes needed? To speed-up the error class migration. The migration on temporary error classes allow us to analyze the errors, so we can detect the most popular error classes. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? ``` $ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite" $ build/sbt "test:testOnly *SQLQuerySuite" $ build/sbt -Phive-thriftserver "hive-thriftserver/testOnly org.apache.spark.sql.hive.thriftserver.ThriftServerQueryTestSuite" ``` Closes apache#38177 from itholic/SPARK-40540-2276-2300. Authored-by: itholic <[email protected]> Signed-off-by: Max Gekk <[email protected]>

[WIP][SPARK-40663][SQL](Final) Migrate execution errors onto error cl…

600ff76

…asses

github-actions bot added CORE SQL labels Oct 9, 2022

itholic added 6 commits October 11, 2022 13:56

resolved conflicts

17a84a9

fix test

72a6f55

resolved comments

ffbb2a6

resolve conflicts

679eb91

resolved conflicts

e6493ba

resolve conflicts

0d77e7e

itholic marked this pull request as ready for review October 18, 2022 13:47

MaxGekk requested changes Oct 18, 2022

View reviewed changes

addressed the comments

f079f0b

Merge branch 'master' of https://github.com/apache/spark into SPARK-4…

db7967d

…0540-2276-2300

itholic changed the title ~~[WIP][SPARK-40663][SQL](Final) Migrate execution errors onto error classes~~ [SPARK-40663][SQL](Final) Migrate execution errors onto error classes Oct 20, 2022

Merge branch 'master' of https://github.com/apache/spark into SPARK-4…

dcb2cb2

…0540-2276-2300

MaxGekk approved these changes Oct 20, 2022

View reviewed changes

itholic added 2 commits October 24, 2022 14:11

fix test

0bf202e

Merge branch 'master' of https://github.com/apache/spark into SPARK-4…

a70608d

…0540-2276-2300

itholic commented Oct 24, 2022

View reviewed changes

itholic added 2 commits October 24, 2022 17:32

Merge branch 'master' of https://github.com/apache/spark into SPARK-4…

1c52bee

…0540-2276-2300

regen the output

7dc3cb6

MaxGekk requested changes Oct 24, 2022

View reviewed changes

itholic added 2 commits October 25, 2022 12:39

add valueType

0b1abdd

fix test

e692011

regen the golden file

9b2ff65

itholic added 2 commits October 25, 2022 16:57

Merge branch 'master' of https://github.com/apache/spark into SPARK-4…

aa24874

…0540-2276-2300

Merge branch 'master' of https://github.com/apache/spark into SPARK-4…

2875329

…0540-2276-2300

MaxGekk closed this in bef9797 Oct 26, 2022

itholic deleted the SPARK-40540-2276-2300 branch April 22, 2023 05:46

		checkExceptionInExpression[SparkException](
		toNumberExpr, "[INTERNAL_ERROR] Undefined error message parameter")

[SPARK-40663][SQL](Final) Migrate execution errors onto error classes #38177

[SPARK-40663][SQL](Final) Migrate execution errors onto error classes #38177

Uh oh!

Conversation

itholic commented Oct 9, 2022

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

itholic commented Oct 18, 2022

Uh oh!

MaxGekk Oct 18, 2022

Choose a reason for hiding this comment

Uh oh!

MaxGekk Oct 18, 2022

Choose a reason for hiding this comment

Uh oh!

itholic commented Oct 19, 2022

Uh oh!

MaxGekk commented Oct 20, 2022

Uh oh!

itholic commented Oct 20, 2022

Uh oh!

MaxGekk left a comment

Choose a reason for hiding this comment

Uh oh!

MaxGekk commented Oct 21, 2022

Uh oh!

MaxGekk commented Oct 24, 2022

Uh oh!

itholic Oct 24, 2022

Choose a reason for hiding this comment

Uh oh!

MaxGekk Oct 24, 2022

Choose a reason for hiding this comment

Uh oh!

itholic Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

MaxGekk Oct 24, 2022

Choose a reason for hiding this comment

Uh oh!

itholic Oct 25, 2022

Choose a reason for hiding this comment

Uh oh!

itholic commented Oct 25, 2022

Uh oh!

MaxGekk commented Oct 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants