Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-46819][CORE] Move error categories and states into JSON #44863

Closed

Conversation

nchammas
Copy link
Contributor

@nchammas nchammas commented Jan 24, 2024

What changes were proposed in this pull request?

Move the error categories and states out of the Markdown table in the error README and into standalone JSON files.

I am using the terms "error category" and "error state" as defined in SPARK-46810.

Why are the changes needed?

The data captured in Markdown is not automation-friendly. In future work, I intend to automate the generation of sql-error-conditions-sqlstates.md using these new JSON files.

Additionally, there are many instances of duplicate keys and inconsistent descriptions for the error categories, which this change resolves by moving the data into JSON. This also enables IDE support for basic validation.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

The updated unit tests ensure the following:

  • All error categories (e.g. 42) and states (e.g. 42K01) have the correct length and format.
  • All error states have a two-character prefix that is a valid error category.
  • All error classes define an error state that is valid.

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

No.

@github-actions github-actions bot added the DOCS label Jan 24, 2024
@nchammas
Copy link
Contributor Author

@MaxGekk - Before I continue down this path, do you like where this is going?

Asking you per @itholic's comment on SPARK-46810.

@itholic
Copy link
Contributor

itholic commented Jan 24, 2024

also cc @cloud-fan

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@github-actions github-actions bot added the CORE label Jan 24, 2024
@nchammas nchammas changed the title [WIP][SPARK-46819] Move error categories and states into JSON [SPARK-46819][CORE] Move error categories and states into JSON Jan 24, 2024
@nchammas
Copy link
Contributor Author

@MaxGekk - The naming choices I made here are based on my proposal in SPARK-46810. Are you OK with what I wrote there? (If yes, could you chime in on the ticket just for the record, please?)

@nchammas nchammas marked this pull request as ready for review January 24, 2024 19:28
@MaxGekk
Copy link
Member

MaxGekk commented Jan 24, 2024

@nchammas I will review this PR tomorrow. Also cc @srielau

@nchammas
Copy link
Contributor Author

No rush at all. (Didn't mean to nag. Dunno if my comment came off that way.)

@srielau
Copy link
Contributor

srielau commented Jan 25, 2024

Thanks for doing this. Adding a row to this table is royal pain in the behind. This will make it easier.

Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nchammas I wonder did you generate error-states.json automatically by a script?

@nchammas
Copy link
Contributor Author

nchammas commented Jan 25, 2024

I wonder did you generate error-states.json automatically by a script?

A little scripting and a little IDE kung-fu.

@nchammas nchammas force-pushed the SPARK-46819-error-data-automation branch from a6d1ecf to 54a1a7e Compare January 25, 2024 16:26
Copy link
Member

@MaxGekk MaxGekk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for CI.

- Redshift.

<!-- SQLSTATE table start -->
|SQLSTATE |Class|Condition |Subclass|Subcondition |Origin |Standard|Used By |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not public doc, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, not public. I will be opening separate PRs to update our public facing documentation on error classes.

@MaxGekk
Copy link
Member

MaxGekk commented Jan 26, 2024

The failure Run / Run Spark on Kubernetes Integration test is not related to the changes, I do believe.

@MaxGekk
Copy link
Member

MaxGekk commented Jan 26, 2024

+1, LGTM. Merging to master.
Thank you, @nchammas and @itholic @srielau for review.

@MaxGekk MaxGekk closed this in 40574bb Jan 26, 2024
@nchammas nchammas deleted the SPARK-46819-error-data-automation branch January 26, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants