Skip to content

[SPARK-39007][SQL] Use double quotes for SQL configs in error messages #36335

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

Closed
wants to merge 5 commits into from

Conversation

MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Apr 24, 2022

What changes were proposed in this pull request?

Wrap SQL configs in error messages by double quotes. Added the toSQLConf() method to QueryErrorsBase to invoke it from Query.*Errors.

Why are the changes needed?

  1. To highlight types and make them more visible for users.
  2. To be able to easily parse types from error text.
  3. To be consistent to other outputs of identifiers, sql statement and etc. where Spark uses quotes or ticks.

Does this PR introduce any user-facing change?

Yes, it changes user-facing error messages.

How was this patch tested?

By running the modified test suites:

$ build/sbt "testOnly *QueryCompilationErrorsSuite"
$ build/sbt "testOnly *QueryExecutionAnsiErrorsSuite"
$ build/sbt "testOnly *QueryExecutionErrorsSuite"

@github-actions github-actions bot added the SQL label Apr 24, 2022
@github-actions github-actions bot added the CORE label Apr 24, 2022
@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 25, 2022

cc @srielau

@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 25, 2022

All GAs have passed already: https://github.com/MaxGekk/spark/runs/6152404822

@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 25, 2022

Merging to master. Thank you, @cloud-fan for review.

@MaxGekk MaxGekk closed this in f01bff9 Apr 25, 2022
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Apr 25, 2022
Wrap SQL configs in error messages by double quotes. Added the `toSQLConf()` method to `QueryErrorsBase` to invoke it from `Query.*Errors`.

1. To highlight types and make them more visible for users.
2. To be able to easily parse types from error text.
3. To be consistent to other outputs of identifiers, sql statement and etc. where Spark uses quotes or ticks.

Yes, it changes user-facing error messages.

By running the modified test suites:
```
$ build/sbt "testOnly *QueryCompilationErrorsSuite"
$ build/sbt "testOnly *QueryExecutionAnsiErrorsSuite"
$ build/sbt "testOnly *QueryExecutionErrorsSuite"
```

Closes apache#36335 from MaxGekk/output-conf-error-class.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit f01bff9)
Signed-off-by: Max Gekk <max.gekk@gmail.com>
MaxGekk added a commit that referenced this pull request Apr 25, 2022
…ssages

### What changes were proposed in this pull request?
Wrap SQL configs in error messages by double quotes. Added the `toSQLConf()` method to `QueryErrorsBase` to invoke it from `Query.*Errors`.

This is a backport of #36335.

### Why are the changes needed?
1. To highlight types and make them more visible for users.
2. To be able to easily parse types from error text.
3. To be consistent to other outputs of identifiers, sql statement and etc. where Spark uses quotes or ticks.

### Does this PR introduce _any_ user-facing change?
Yes, it changes user-facing error messages.

### How was this patch tested?
By running the modified test suites:
```
$ build/sbt "testOnly *QueryCompilationErrorsSuite"
$ build/sbt "testOnly *QueryExecutionAnsiErrorsSuite"
$ build/sbt "testOnly *QueryExecutionErrorsSuite"
```

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

Closes #36340 from MaxGekk/output-conf-error-class-3.3.

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>
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.

2 participants