[SPARK-43259][SQL] Assign a name to the error class _LEGACY_ERROR_TEMP_2024 #45095

mihailom-db · 2024-02-14T09:16:06Z

What changes were proposed in this pull request?

In the PR, I propose to assign the proper name INVALID_EXPRESSION_ENCODER to the legacy error class _LEGACY_ERROR_TEMP_2024, and add a test to the suite which uses checkError(). Also this PR improves the error message.

Why are the changes needed?

Proper name improves user experience w/ Spark SQL.

Does this PR introduce any user-facing change?

Yes, the PR changes an user-facing error message.

How was this patch tested?

By running the modified test suite:

./build/mvn -Dtest=none -DwildcardSuites=org.apache.spark.sql.errors.QueryExecutionErrorsSuite test

sql/core/src/test/scala/org/apache/spark/sql/errors/QueryExecutionErrorsSuite.scala

MaxGekk

Please, assign a proper name for the error class, and add a test using public API.

MaxGekk · 2024-02-20T09:00:52Z

In the PR, I propose to assign the proper name UNSUPPORTED_ERROR ...

@mihailom-db Did you really assign this name?

MaxGekk · 2024-02-20T09:03:25Z

@mihailom-db BTW, do you have an account at OSS JIRA? If so, could you leave a comment in https://issues.apache.org/jira/browse/SPARK-43259 - telling that you are working on this.

mihailom-db · 2024-02-20T09:11:08Z

Requested the access for the OSS JIRA account, should be able to comment soon.

common/utils/src/main/resources/error/error-classes.json

common/utils/src/main/resources/error/error-states.json

MaxGekk · 2024-02-22T07:10:08Z

common/utils/src/main/resources/error/error-states.json

+        "description": "Invalid encoder error",
+        "origin": "SQL/Foundation",
+        "standard": "Y",
+        "usedBy": ["SQL/Foundation"]


Could you check is this SQL state used by other DBMS?

I took a look at different DBMS sqlstates and to my knowledge it seems that this one is not used by any. Changed doc accordingly.

MaxGekk · 2024-02-22T17:31:31Z

+1, LGTM. Merging to master.
Thank you, @mihailom-db.

MaxGekk · 2024-02-22T17:34:12Z

@mihailom-db Congratulations with your first contribution to Apache Spark!

dongjoon-hyun

Hi, @mihailom-db and @MaxGekk

Does this PR pass the CIs? It seem to break Apache Spark master branch. Could you take a look at the failure?

dongjoon-hyun · 2024-02-23T01:16:52Z

https://github.com/apache/spark/actions/runs/8008631301/job/21875499011

[info] SparkThrowableSuite:
[info] - No duplicate error classes (20 milliseconds)
[info] - Error classes are correctly formatted (35 milliseconds)
[info] - SQLSTATE is mandatory (2 milliseconds)
[info] - Error category and error state / SQLSTATE invariants (23 milliseconds)
[info] - Message invariants (7 milliseconds)
[info] - Message format invariants (8 milliseconds)
[info] - Error classes match with document *** FAILED *** (55 milliseconds)
[info]   "...ects an instance of [ExpressionEncoder] but got `<encoderTy..." did not equal "...ects an instance of [`ExpressionEncoder`] but got `<encoderTy..." The error class document is not up to date. Please regenerate it. (SparkThrowableSuite.scala:353)
[info]   Analysis:
[info]   "...ects an instance of [ExpressionEncoder] but got `<encoderTy..." -> "...ects an instance of [`ExpressionEncoder`] but got `<encoderTy..."

dongjoon-hyun · 2024-02-23T01:17:17Z

NVM. Let me make a quick followup.

dongjoon-hyun · 2024-02-23T01:19:18Z

Here is a follow-up.

[SPARK-43259][SQL][FOLLOWUP] Regenerate sql-error-conditions.md to recover SparkThrowableSuite #45226

…recover `SparkThrowableSuite` ### What changes were proposed in this pull request? This is a follow-up of #45095 ### Why are the changes needed? To recover the broken `master` branch. - https://github.com/apache/spark/actions/runs/8008631301/job/21875499011 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. I manually verified like the following. ``` [info] SparkThrowableSuite: [info] - No duplicate error classes (23 milliseconds) [info] - Error classes are correctly formatted (37 milliseconds) [info] - SQLSTATE is mandatory (1 millisecond) [info] - Error category and error state / SQLSTATE invariants (21 milliseconds) [info] - Message invariants (6 milliseconds) [info] - Message format invariants (9 milliseconds) [info] - Error classes match with document (54 milliseconds) [info] - Round trip (23 milliseconds) [info] - Error class names should contain only capital letters, numbers and underscores (5 milliseconds) [info] - Check if error class is missing (14 milliseconds) [info] - Check if message parameters match message format (2 milliseconds) [info] - Error message is formatted (0 milliseconds) [info] - Error message does not do substitution on values (0 milliseconds) [info] - Try catching legacy SparkError (1 millisecond) [info] - Try catching SparkError with error class (1 millisecond) [info] - Try catching internal SparkError (1 millisecond) [info] - Get message in the specified format (3 milliseconds) [info] - overwrite error classes (47 milliseconds) [info] - prohibit dots in error class names (15 milliseconds) [info] Run completed in 1 second, 90 milliseconds. [info] Total number of tests run: 19 [info] Suites: completed 1, aborted 0 [info] Tests: succeeded 19, failed 0, canceled 0, ignored 0, pending 0 [info] All tests passed. [success] Total time: 7 s, completed Feb 22, 2024, 5:22:24 PM ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes #45226 from dongjoon-hyun/SPARK-43259. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

…P_2024 ### What changes were proposed in this pull request? In the PR, I propose to assign the proper name `INVALID_EXPRESSION_ENCODER` to the legacy error class `_LEGACY_ERROR_TEMP_2024`, and add a test to the suite which uses `checkError()`. Also this PR improves the error message. ### Why are the changes needed? Proper name improves user experience w/ Spark SQL. ### Does this PR introduce _any_ user-facing change? Yes, the PR changes an user-facing error message. ### How was this patch tested? By running the modified test suite: ``` ./build/mvn -Dtest=none -DwildcardSuites=org.apache.spark.sql.errors.QueryExecutionErrorsSuite test ``` Closes apache#45095 from mihailom-db/SPARK-43259. Authored-by: Mihailo Milosevic <[email protected]> Signed-off-by: Max Gekk <[email protected]>

…recover `SparkThrowableSuite` ### What changes were proposed in this pull request? This is a follow-up of apache#45095 ### Why are the changes needed? To recover the broken `master` branch. - https://github.com/apache/spark/actions/runs/8008631301/job/21875499011 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. I manually verified like the following. ``` [info] SparkThrowableSuite: [info] - No duplicate error classes (23 milliseconds) [info] - Error classes are correctly formatted (37 milliseconds) [info] - SQLSTATE is mandatory (1 millisecond) [info] - Error category and error state / SQLSTATE invariants (21 milliseconds) [info] - Message invariants (6 milliseconds) [info] - Message format invariants (9 milliseconds) [info] - Error classes match with document (54 milliseconds) [info] - Round trip (23 milliseconds) [info] - Error class names should contain only capital letters, numbers and underscores (5 milliseconds) [info] - Check if error class is missing (14 milliseconds) [info] - Check if message parameters match message format (2 milliseconds) [info] - Error message is formatted (0 milliseconds) [info] - Error message does not do substitution on values (0 milliseconds) [info] - Try catching legacy SparkError (1 millisecond) [info] - Try catching SparkError with error class (1 millisecond) [info] - Try catching internal SparkError (1 millisecond) [info] - Get message in the specified format (3 milliseconds) [info] - overwrite error classes (47 milliseconds) [info] - prohibit dots in error class names (15 milliseconds) [info] Run completed in 1 second, 90 milliseconds. [info] Total number of tests run: 19 [info] Suites: completed 1, aborted 0 [info] Tests: succeeded 19, failed 0, canceled 0, ignored 0, pending 0 [info] All tests passed. [success] Total time: 7 s, completed Feb 22, 2024, 5:22:24 PM ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#45226 from dongjoon-hyun/SPARK-43259. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

github-actions bot added the SQL label Feb 14, 2024

mihailom-db force-pushed the SPARK-43259 branch from 3fa05cf to 6174cb7 Compare February 14, 2024 09:18

mihailom-db changed the title ~~[SPARK-43259][SQL] Assign a name to the error class _LEGACY_ERROR_TEMP_2024~~ [SPARK-43259][SQL] Assign a name to the error class _LEGACY_ERROR_TEMP_2024 [WIP] Feb 14, 2024

github-actions bot added the DOCS label Feb 14, 2024

mihailom-db changed the title ~~[SPARK-43259][SQL] Assign a name to the error class _LEGACY_ERROR_TEMP_2024 [WIP]~~ [SPARK-43259][SQL] Assign a name to the error class _LEGACY_ERROR_TEMP_2024 Feb 14, 2024

MaxGekk reviewed Feb 14, 2024

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/errors/QueryExecutionErrorsSuite.scala Outdated Show resolved Hide resolved

github-actions bot removed the DOCS label Feb 19, 2024

mihailom-db requested a review from MaxGekk February 19, 2024 11:46

MaxGekk requested changes Feb 19, 2024

View reviewed changes

github-actions bot added the DOCS label Feb 20, 2024

mihailom-db requested a review from MaxGekk February 20, 2024 08:27

MaxGekk reviewed Feb 20, 2024

View reviewed changes

common/utils/src/main/resources/error/error-classes.json Outdated Show resolved Hide resolved

common/utils/src/main/resources/error/error-classes.json Outdated Show resolved Hide resolved

MaxGekk requested changes Feb 21, 2024

View reviewed changes

common/utils/src/main/resources/error/error-classes.json Outdated Show resolved Hide resolved

mihailom-db requested a review from MaxGekk February 21, 2024 10:20

MaxGekk reviewed Feb 21, 2024

View reviewed changes

common/utils/src/main/resources/error/error-classes.json Outdated Show resolved Hide resolved

common/utils/src/main/resources/error/error-classes.json Outdated Show resolved Hide resolved

common/utils/src/main/resources/error/error-classes.json Outdated Show resolved Hide resolved

mihailom-db commented Feb 21, 2024

View reviewed changes

common/utils/src/main/resources/error/error-states.json Outdated Show resolved Hide resolved

mihailom-db requested a review from MaxGekk February 21, 2024 20:24

mihailom-db added 10 commits February 22, 2024 06:52

_LEGACY_ERROR_TEMP_2024 renaming

04ef307

Fix ordering in error-classes.json

7aefa0f

Add UNSUPPORTED_ENCODER to md files

79e9f95

Swap to Internal Error

e282f7f

Fix deleted imports

de30401

Reproduce error with public API

595f969

Change error sqlstate and fix error message

abdea85

Fixing Docs Reference

e531110

Improve error naming

a88a8f8

rerun checks

344f73f

mihailom-db force-pushed the SPARK-43259 branch from 7428b30 to 344f73f Compare February 22, 2024 05:56

Fix scala style error

9b799ca

MaxGekk reviewed Feb 22, 2024

View reviewed changes

Fix error state documentation

3fa1f0d

mihailom-db requested a review from MaxGekk February 22, 2024 11:29

MaxGekk approved these changes Feb 22, 2024

View reviewed changes

MaxGekk closed this in 6de527e Feb 22, 2024

dongjoon-hyun reviewed Feb 23, 2024

View reviewed changes

dongjoon-hyun mentioned this pull request Feb 23, 2024

[SPARK-43259][SQL][FOLLOWUP] Regenerate sql-error-conditions.md to recover SparkThrowableSuite #45226

Closed

mihailom-db deleted the SPARK-43259 branch February 27, 2024 13:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-43259][SQL] Assign a name to the error class _LEGACY_ERROR_TEMP_2024 #45095

[SPARK-43259][SQL] Assign a name to the error class _LEGACY_ERROR_TEMP_2024 #45095

mihailom-db commented Feb 14, 2024 •

edited

Loading

MaxGekk left a comment

MaxGekk commented Feb 20, 2024

MaxGekk commented Feb 20, 2024

mihailom-db commented Feb 20, 2024

MaxGekk Feb 22, 2024

mihailom-db Feb 22, 2024

MaxGekk commented Feb 22, 2024

MaxGekk commented Feb 22, 2024

dongjoon-hyun left a comment

dongjoon-hyun commented Feb 23, 2024 •

edited

Loading

dongjoon-hyun commented Feb 23, 2024

dongjoon-hyun commented Feb 23, 2024

[SPARK-43259][SQL] Assign a name to the error class _LEGACY_ERROR_TEMP_2024 #45095

[SPARK-43259][SQL] Assign a name to the error class _LEGACY_ERROR_TEMP_2024 #45095

Conversation

mihailom-db commented Feb 14, 2024 • edited Loading

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?

MaxGekk left a comment

Choose a reason for hiding this comment

MaxGekk commented Feb 20, 2024

MaxGekk commented Feb 20, 2024

mihailom-db commented Feb 20, 2024

MaxGekk Feb 22, 2024

Choose a reason for hiding this comment

mihailom-db Feb 22, 2024

Choose a reason for hiding this comment

MaxGekk commented Feb 22, 2024

MaxGekk commented Feb 22, 2024

dongjoon-hyun left a comment

Choose a reason for hiding this comment

dongjoon-hyun commented Feb 23, 2024 • edited Loading

dongjoon-hyun commented Feb 23, 2024

dongjoon-hyun commented Feb 23, 2024

mihailom-db commented Feb 14, 2024 •

edited

Loading

dongjoon-hyun commented Feb 23, 2024 •

edited

Loading