Skip to content

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 <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
(cherry picked from commit f01bff9)
Signed-off-by: Max Gekk <[email protected]>
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 <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
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 <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
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 <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
(cherry picked from commit 2a4d8a4)
Signed-off-by: Max Gekk <[email protected]>
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