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-38913][SQL][3.3] Output identifiers in error messages in SQL style #36288

Closed
wants to merge 2 commits into from

Conversation

MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Apr 20, 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"

Authored-by: Max Gekk max.gekk@gmail.com
Signed-off-by: Max Gekk max.gekk@gmail.com
(cherry picked from commit 2ff6914)

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM if tests pass

@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 21, 2022

GAs passed. Merging to 3.3.
Thank you, @HyukjinKwon for review.

MaxGekk added a commit that referenced this pull request Apr 21, 2022
…tyle

### 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:
```sql
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"
```

Authored-by: Max Gekk <max.gekkgmail.com>
Signed-off-by: Max Gekk <max.gekkgmail.com>
(cherry picked from commit 2ff6914)

Closes #36288 from MaxGekk/error-class-toSQLId-3.3.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
@MaxGekk MaxGekk closed this Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants