Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Apr 15, 2022

What changes were proposed in this pull request?

In the PR, I propose to use backticks to wrap SQL identifiers in error messages. I added new util functions toSQLId() to the trait QueryErrorsBase, and applied it in Query.*Errors (also modified tests in Query.*ErrorsSuite). For example:

Before:

Invalid SQL syntax: The definition of window win is repetitive.

After:

Invalid SQL syntax: The definition of window `win` is repetitive.

Why are the changes needed?

To improve user experience with Spark SQL. The changes highlight SQL identifiers in error massages and make them more visible for users.

Does this PR introduce any user-facing change?

No since error classes haven't been released yet.

How was this patch tested?

By running the affected test suites:

$ build/sbt "test:testOnly *QueryParsingErrorsSuite"
$ build/sbt "test:testOnly *QueryCompilationErrorsSuite"
$ build/sbt "test:testOnly *QueryCompilationErrorsDSv2Suite"
$ build/sbt "test:testOnly *QueryExecutionErrorsSuite"
$ build/sbt "testOnly *PlanParserSuite"
$ build/sbt "testOnly *DDLParserSuite"
$ build/sbt -Phive-2.3 "testOnly *HiveSQLInsertTestSuite"
$ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z window.sql"
$ build/sbt "testOnly *DSV2SQLInsertTestSuite"

@MaxGekk MaxGekk changed the title [WIP][SPARK-38913][SQL] Output identifiers in error messages in SQL style [SPARK-38913][SQL] Output identifiers in error messages in SQL style Apr 15, 2022
@MaxGekk MaxGekk marked this pull request as ready for review April 15, 2022 17:52
@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 15, 2022

@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 15, 2022

@ivoson @yutoacts @leesf @cloud-fan @HyukjinKwon @gengliangwang @panbingkun Could you review this PR, please.

@ivoson
Copy link
Contributor

ivoson commented Apr 17, 2022

LGTM

@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 17, 2022

Merging to master. Thank you, @leesf and @HyukjinKwon for review.

@MaxGekk MaxGekk closed this in 2ff6914 Apr 17, 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.

4 participants