[SPARK-46810][DOCS] Align error class terminology with SQL standard #44902

nchammas · 2024-01-26T15:44:37Z

What changes were proposed in this pull request?

Clarify the error class terminology in our internal errors README per the proposal in SPARK-46810.
Rename error-classes.json to error-conditions.json and update the codebase accordingly.
Rename error-categories.json to error-classes.json and update the codebase accordingly.
Improve the formatting of the code snippets in the errors README.

Why are the changes needed?

We should use error class terminology consistently and unambiguously, and we should stick as closely to the SQL standard as possible.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing unit tests.

Was this patch authored or co-authored using generative AI tooling?

No.

common/utils/src/main/resources/error/README.md

nchammas · 2024-01-26T15:45:49Z

cc @MaxGekk and @srielau

nchammas · 2024-03-14T01:05:22Z

@MaxGekk - Given the recent discussion on SPARK-46810, shall I expand this PR to include renaming errorClass across the codebase? Or would you like to break that into its own PR?

MaxGekk · 2024-03-14T05:57:05Z

Or would you like to break that into its own PR?

Let's do that in a separate PR because I expect massive changes. It should be easier to review only one kind of changes.

nchammas · 2024-03-17T00:26:40Z

I've created SPARK-47429 to track renaming errorClass to errorCondition.

The build is passing, and this is ready for review, @MaxGekk.

nchammas · 2024-03-18T21:53:28Z

Since error-classes.json is frequently updated, it will repeatedly cause merge conflicts here. I will refrain from resolving those merge conflicts until this PR is actively under review.

nchammas · 2024-03-31T15:46:28Z

@MaxGekk - Is there something I can do to move this along?

Once this is merged in, I am happy to work on SPARK-47429, as I noted in the comments there. (But it's also fine if you'd prefer to work on it yourself.)

srielau · 2024-04-01T14:23:50Z

@MaxGekk - Is there something I can do to move this along?

Once this is merged in, I am happy to work on SPARK-47429, as I noted in the comments there. (But it's also fine if you'd prefer to work on it yourself.)

@nchammas Not sure if we can count on a response from @MaxGekk right now. @cloud-fan How can we proceed here?

cloud-fan · 2024-04-15T02:13:37Z

The JSON field still has "subClass", do we need to rename it?

nchammas · 2024-04-15T13:51:45Z

The JSON field still has "subClass", do we need to rename it?

I was planning to change that as part of SPARK-47429, since that affects the internal API and will likely require a large diff. If you agree with that, I will update the description for SPARK-47429 accordingly.

cloud-fan · 2024-04-15T14:09:30Z

oh, sounds good to do it later. Let's rebase this PR and then merge it ASAP.

nchammas · 2024-04-15T18:49:18Z

@cloud-fan - Ready for review.

cloud-fan · 2024-04-16T02:36:37Z

thanks, merging to master!

panbingkun · 2024-04-17T11:48:49Z

common/utils/src/main/resources/error/README.md

+
+Fixing this will require renaming `SparkException.errorClass` to `SparkException.errorCondition` and making similar changes to `ErrorClassesJsonReader` and other parts of the codebase. We will address this in [SPARK-47429]. Until that is complete, we will have to live with the fact that a string like `DATATYPE_MISSING_SIZE` is called an "error condition" in our user-facing documentation but an "error class" in the code.
+
+For more details, please see [SPARK-46810][SPARK-46810].


Why do we need to write [SPARK-46810] twice here? Is it because of the special syntax for markdown? Or do we actually want to write: [SPARK-46810] [SPARK-47429]

Nope, I meant to link to SPARK-46810. The first [SPARK-46810] is the text I want to link, and the second [SPARK-46810] is the reference to the link URL I have defined just below in the same document:

[SPARK-46810]: https://issues.apache.org/jira/browse/SPARK-46810

### What changes were proposed in this pull request? The pr is following up #44902, to make some `reference files links` clickable. ### Why are the changes needed? Convenient for developers to navigate directly when read this file `README.md`. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manually test. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #46105 from panbingkun/SPARK-46810_FOLLOWUP. Authored-by: panbingkun <panbingkun@baidu.com> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…kError()` ### What changes were proposed in this pull request? In the PR, I propose to rename the `errorClass` parameter of the `checkError()` functions (and related and similar functions) to `condition` to follow the naming convention introduced by SPARK-46810. For example: ```scala checkError( exception = e, condition = "UNRESOLVED_COLUMN.WITH_SUGGESTION", parameters = Map("objectName" -> "`non_exist`", "proposal" -> "`i`, `j`")) ``` ### Why are the changes needed? To follow new naming convention introduced by #44902. ### Does this PR introduce _any_ user-facing change? No, the changes affect **only in tests**. ### How was this patch tested? By running the existing test suites and GAs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #48027 from MaxGekk/rename-checkError-condition. Authored-by: Max Gekk <max.gekk@gmail.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>

…kError()` ### What changes were proposed in this pull request? In the PR, I propose to rename the `errorClass` parameter of the `checkError()` functions (and related and similar functions) to `condition` to follow the naming convention introduced by SPARK-46810. For example: ```scala checkError( exception = e, condition = "UNRESOLVED_COLUMN.WITH_SUGGESTION", parameters = Map("objectName" -> "`non_exist`", "proposal" -> "`i`, `j`")) ``` ### Why are the changes needed? To follow new naming convention introduced by apache#44902. ### Does this PR introduce _any_ user-facing change? No, the changes affect **only in tests**. ### How was this patch tested? By running the existing test suites and GAs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#48027 from MaxGekk/rename-checkError-condition. Authored-by: Max Gekk <max.gekk@gmail.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>

explain terminology

1c6184a

github-actions bot added the DOCS label Jan 26, 2024

nchammas commented Jan 26, 2024

View reviewed changes

common/utils/src/main/resources/error/README.md Outdated Show resolved Hide resolved

nchammas added 3 commits January 28, 2024 14:30

update error README per discussion on SPARK-46810

e9e072e

error-classes.json -> error-conditions.json

6537240

error-categories.json -> error-classes.json

5684213

github-actions bot added SQL STRUCTURED STREAMING CORE labels Jan 28, 2024

nchammas added 2 commits January 28, 2024 14:53

wording tweaks

b2b9acf

remove todo

e3ce5ec

nchammas changed the title ~~[SPARK-46810][DOCS] Explain error class terminology in internal README~~ [SPARK-46810][DOCS] Align error class terminology with SQL standard Jan 28, 2024

nchammas mentioned this pull request Jan 29, 2024

[SPARK-46894][PYTHON] Move PySpark error conditions into standalone JSON file #44920

Closed

nchammas added 4 commits March 16, 2024 12:11

Merge branch 'master' into SPARK-46810-error-class-terminology

f460dd4

update conditions from master

483c97d

update comments to anticipate SPARK-47429

ab3838d

restore missing brace

6929187

nchammas added 2 commits April 15, 2024 10:31

Merge branch 'master' into SPARK-46810-error-class-terminology

b042fef

update error-conditions from master

c7a83ed

cloud-fan approved these changes Apr 16, 2024

View reviewed changes

cloud-fan closed this in c5b8e60 Apr 16, 2024

nchammas deleted the SPARK-46810-error-class-terminology branch April 16, 2024 16:00

panbingkun reviewed Apr 17, 2024

View reviewed changes

panbingkun mentioned this pull request Apr 17, 2024

[SPARK-46810][DOCS][FOLLOWUP] Make some reference file links clickable #46105

Closed

MaxGekk mentioned this pull request Sep 8, 2024

[SPARK-49579][SQL][TESTS] Rename errorClass to condition in checkError() #48027

Closed

This was referenced Oct 12, 2024

[SPARK-49941][CORE] Rename errorClass to condition in errors of the JSON format #48431

Open

[SPARK-49942][SQL] Rename errorClass to condition in classifyException() #48433

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-46810][DOCS] Align error class terminology with SQL standard #44902

[SPARK-46810][DOCS] Align error class terminology with SQL standard #44902

nchammas commented Jan 26, 2024 •

edited

Loading

nchammas commented Jan 26, 2024

nchammas commented Mar 14, 2024

MaxGekk commented Mar 14, 2024

nchammas commented Mar 17, 2024

nchammas commented Mar 18, 2024

nchammas commented Mar 31, 2024

srielau commented Apr 1, 2024

cloud-fan commented Apr 15, 2024

nchammas commented Apr 15, 2024

cloud-fan commented Apr 15, 2024

nchammas commented Apr 15, 2024

cloud-fan commented Apr 16, 2024

panbingkun Apr 17, 2024

nchammas Apr 17, 2024


		Fixing this will require renaming `SparkException.errorClass` to `SparkException.errorCondition` and making similar changes to `ErrorClassesJsonReader` and other parts of the codebase. We will address this in [SPARK-47429]. Until that is complete, we will have to live with the fact that a string like `DATATYPE_MISSING_SIZE` is called an "error condition" in our user-facing documentation but an "error class" in the code.

		For more details, please see [SPARK-46810][SPARK-46810].

[SPARK-46810][DOCS] Align error class terminology with SQL standard #44902

[SPARK-46810][DOCS] Align error class terminology with SQL standard #44902

Conversation

nchammas commented Jan 26, 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?

Was this patch authored or co-authored using generative AI tooling?

nchammas commented Jan 26, 2024

nchammas commented Mar 14, 2024

MaxGekk commented Mar 14, 2024

nchammas commented Mar 17, 2024

nchammas commented Mar 18, 2024

nchammas commented Mar 31, 2024

srielau commented Apr 1, 2024

cloud-fan commented Apr 15, 2024

nchammas commented Apr 15, 2024

cloud-fan commented Apr 15, 2024

nchammas commented Apr 15, 2024

cloud-fan commented Apr 16, 2024

panbingkun Apr 17, 2024

Choose a reason for hiding this comment

nchammas Apr 17, 2024

Choose a reason for hiding this comment

nchammas commented Jan 26, 2024 •

edited

Loading