Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Apr 26, 2022

What changes were proposed in this pull request?

In the PR, I propose to output any SQL statement in error messages in upper case, and apply new implementation of QueryErrorsBase.toSQLStmt() to all exceptions in Query.*Errors w/ error classes. Also this PR modifies all affected tests, see the list in the section "How was this patch tested?".

Why are the changes needed?

To improve user experience with Spark SQL by highlighting SQL statements in error massage and make them more visible to users. Also this PR makes SQL statements in error messages consistent to the docs where such elements are showed in upper case w/ any quotes.

Does this PR introduce any user-facing change?

Yes. The changes might influence on error messages:

Before:

The operation "DESC PARTITION" is not allowed

After:

The operation DESC PARTITION is not allowed

How was this patch tested?

By running affected test suites:

$ build/sbt "sql/testOnly *QueryExecutionErrorsSuite"
$ build/sbt "sql/testOnly *QueryParsingErrorsSuite"
$ build/sbt "sql/testOnly *QueryCompilationErrorsSuite"
$ build/sbt "test:testOnly *QueryCompilationErrorsDSv2Suite"
$ build/sbt "test:testOnly *ExtractPythonUDFFromJoinConditionSuite"
$ build/sbt "testOnly *PlanParserSuite"
$ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z transform.sql"
$ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z join-lateral.sql"
$ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z describe.sql"
$ build/sbt "testOnly *DDLParserSuite"

@github-actions github-actions bot added the SQL label Apr 26, 2022
@MaxGekk MaxGekk changed the title [WIP][SQL] Output SQL statements in upper case in error messages [SPARK-39027][SQL] Output SQL statements in upper case in error messages Apr 26, 2022
@MaxGekk MaxGekk marked this pull request as ready for review April 26, 2022 15:34
@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 26, 2022

@cloud-fan @srielau @HyukjinKwon Could you approve remove double quotes around SQL statements in error messages, please.

@MaxGekk MaxGekk changed the title [SPARK-39027][SQL] Output SQL statements in upper case in error messages [SPARK-39027][SQL] Output SQL statements in error messages in upper case and w/o double quotes Apr 26, 2022
@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 26, 2022

cc @srielau

dongjoon-hyun pushed a commit that referenced this pull request Apr 27, 2022
…per case and w/o double quotes

### What changes were proposed in this pull request?
In the PR, I propose to output any SQL statement in error messages in upper case, and apply new implementation of `QueryErrorsBase.toSQLStmt()` to all exceptions in `Query.*Errors` w/ error classes. Also this PR modifies all affected tests, see the list in the section "How was this patch tested?".

This is a backport of #36359.

### Why are the changes needed?
To improve user experience with Spark SQL by highlighting SQL statements in error massage and make them more visible to users. Also this PR makes SQL statements in error messages consistent to the docs where such elements are showed in upper case w/ any quotes.

### Does this PR introduce _any_ user-facing change?
Yes. The changes might influence on error messages:

Before:
```sql
The operation "DESC PARTITION" is not allowed
```

After:
```sql
The operation DESC PARTITION is not allowed
```

### How was this patch tested?
By running affected test suites:
```
$ build/sbt "sql/testOnly *QueryExecutionErrorsSuite"
$ build/sbt "sql/testOnly *QueryParsingErrorsSuite"
$ build/sbt "sql/testOnly *QueryCompilationErrorsSuite"
$ build/sbt "test:testOnly *QueryCompilationErrorsDSv2Suite"
$ build/sbt "test:testOnly *ExtractPythonUDFFromJoinConditionSuite"
$ build/sbt "testOnly *PlanParserSuite"
$ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z transform.sql"
$ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z join-lateral.sql"
$ build/sbt "sql/testOnly *SQLQueryTestSuite -- -z describe.sql"
$ build/sbt "testOnly *DDLParserSuite"
```

Closes #36363 from MaxGekk/error-class-toSQLStmt-no-quotes-3.3.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 27, 2022

All GAs passed: https://github.com/MaxGekk/spark/runs/6189050094. Merging to master.
Thank you, @HyukjinKwon @cloud-fan for review.

@MaxGekk MaxGekk closed this in 4e84f33 Apr 27, 2022
MaxGekk added a commit that referenced this pull request May 22, 2022
### What changes were proposed in this pull request?
In the PR, I propose to describe the rules of quoting elements in error messages introduced by the PRs:
- #36210
- #36233
- #36259
- #36324
- #36335
- #36359
- #36579

### Why are the changes needed?
To improve code maintenance, and the process of code review.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
By existing GAs.

Closes #36621 from MaxGekk/update-error-class-guide.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
MaxGekk added a commit that referenced this pull request May 22, 2022
### What changes were proposed in this pull request?
In the PR, I propose to describe the rules of quoting elements in error messages introduced by the PRs:
- #36210
- #36233
- #36259
- #36324
- #36335
- #36359
- #36579

### Why are the changes needed?
To improve code maintenance, and the process of code review.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
By existing GAs.

Closes #36621 from MaxGekk/update-error-class-guide.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit 2a4d8a4)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants