Skip to content

Comments

[SPARK-38732][SQL][TESTS] Test the error class: INCOMPARABLE_PIVOT_COLUMN#36320

Closed
lvshaokang wants to merge 6 commits intoapache:masterfrom
lvshaokang:SPARK-38732
Closed

[SPARK-38732][SQL][TESTS] Test the error class: INCOMPARABLE_PIVOT_COLUMN#36320
lvshaokang wants to merge 6 commits intoapache:masterfrom
lvshaokang:SPARK-38732

Conversation

@lvshaokang
Copy link
Contributor

What changes were proposed in this pull request?

I add a test case for the error class INCOMPARABLE_PIVOT_COLUMN in the QueryExecutionErrorsSuite.

Why are the changes needed?

To improve test coverage, and record expected error messages in tests.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

By existing tests.

lvshaokang and others added 2 commits April 22, 2022 12:59
### What changes were proposed in this pull request?
In the PR, I propose to prepend the error class tag to error messages. The `SparkThrowableHelper.getMessage()` method was modified for this purpose:
```scala
def getMessage(errorClass: String, messageParameters: Array[String]): String = {
  ...
    "[" + errorClass + "] " + String.format ...
  }
```

Other changes are related to tests where the expected error messages were extended by error class tags. Also I introduced new base trait `QueryErrorsSuiteBase` for test suites w/ two helper methods:
```scala
  def checkErrorClass(
      exception: Exception with SparkThrowable,
      errorClass: String,
      msg: String,
      sqlState: Option[String] = None,
      matchMsg: Boolean = false): Unit

  def validateParsingError(
      sqlText: String,
      errorClass: String,
      sqlState: String,
      message: String): Unit
```
The methods are supposed to be used in `QueryParsingErrorsSuite`, `QueryCompilationErrorsSuite`, `QueryExecutionErrorsSuite` and so on, to check error classes, messages, and sqlStates.

### Why are the changes needed?
The error class tags will be used as unique identifiers to google issues, and refer to Spark docs.

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

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

Closes apache#36287 from MaxGekk/output-error-class-tag.

Authored-by: Max Gekk <max.gekk@gmail.com>
Signed-off-by: Max Gekk <max.gekk@gmail.com>
@github-actions github-actions bot added the SQL label Apr 22, 2022
### What changes were proposed in this pull request?

This PR proposes to add an example of `StreamingQueryListener` in Python addd in SPARK-38759.

### Why are the changes needed?

To guide users about the new feature `StreamingQueryListener`.

### Does this PR introduce _any_ user-facing change?

Yes, new example in the documentation:

<img width="947" alt="Screen Shot 2022-04-22 at 2 01 36 PM" src="https://user-images.githubusercontent.com/6477701/164606564-2aaeb912-7c09-463d-a567-d4d52cab8c97.png">

### How was this patch tested?

Manually built the docs and checked the output.

Closes apache#36318 from HyukjinKwon/SPARK-38994.

Authored-by: Hyukjin Kwon <gurwls223@apache.org>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
Comment on lines +395 to +398
assert(e.getErrorClass === "INCOMPARABLE_PIVOT_COLUMN")
assert(e.getSqlState === "42000")
assert(e.getMessage.matches("Invalid pivot column 'map.*\\'. " +
"Pivot columns must be comparable."))
Copy link
Member

Choose a reason for hiding this comment

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

Could you use the checkErrorClass method added by #36287. BTW, the method checks the tag in error messages.

@lvshaokang lvshaokang closed this Apr 22, 2022
@lvshaokang lvshaokang deleted the SPARK-38732 branch April 22, 2022 08:53
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.

3 participants