Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Apr 17, 2022

What changes were proposed in this pull request?

In the PR, I propose to upper case SQL types in error messages similar to the SQL standard. I added new util functions toSQLType() to the trait QueryErrorsBase, and applied it in Query.*Errors (also modified tests in Query.*ErrorsSuite). For example:

Before:

Cannot up cast b.`b` from decimal(38,18) to bigint.

After:

Cannot up cast b.`b` from DECIMAL(38,18) to BIGINT.

Why are the changes needed?

To improve user experience with Spark SQL. The changes highlight SQL types 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 modified test suites:

$ build/sbt "test:testOnly *QueryParsingErrorsSuite"
$ build/sbt "test:testOnly *QueryCompilationErrorsSuite"
$ build/sbt "test:testOnly *QueryExecutionErrorsSuite"
$ build/sbt "testOnly *CastSuite"
$ build/sbt "testOnly *AnsiCastSuiteWithAnsiModeOn"
$ build/sbt "testOnly *EncoderResolutionSuite"
$ build/sbt "test:testOnly *DatasetSuite"
$ build/sbt "test:testOnly *InsertSuite"

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

MaxGekk commented Apr 17, 2022

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

MaxGekk commented Apr 17, 2022

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

@MaxGekk
Copy link
Member Author

MaxGekk commented Apr 18, 2022

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

@MaxGekk MaxGekk closed this in 0d16159 Apr 18, 2022
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Apr 18, 2022
In the PR, I propose to upper case SQL types in error messages similar to the SQL standard. I added new util functions `toSQLType()` to the trait `QueryErrorsBase`, and applied it in `Query.*Errors` (also modified tests in `Query.*ErrorsSuite`). For example:

Before:
```sql
Cannot up cast b.`b` from decimal(38,18) to bigint.
```

After:
```sql
Cannot up cast b.`b` from DECIMAL(38,18) to BIGINT.
```

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

No since error classes haven't been released yet.

By running the modified test suites:
```
$ build/sbt "test:testOnly *QueryParsingErrorsSuite"
$ build/sbt "test:testOnly *QueryCompilationErrorsSuite"
$ build/sbt "test:testOnly *QueryExecutionErrorsSuite"
$ build/sbt "testOnly *CastSuite"
$ build/sbt "testOnly *AnsiCastSuiteWithAnsiModeOn"
$ build/sbt "testOnly *EncoderResolutionSuite"
$ build/sbt "test:testOnly *DatasetSuite"
$ build/sbt "test:testOnly *InsertSuite"
```

Closes apache#36233 from MaxGekk/error-class-toSQLType.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
(cherry picked from commit 0d16159)
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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants