Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Sep 8, 2024

What changes were proposed in this pull request?

In the PR, I propose to rename the errorClass parameter of the checkError() functions (and related and similar functions) to condition to follow the naming convention introduced by SPARK-46810.

For example:

checkError(
  exception = e,
  condition = "UNRESOLVED_COLUMN.WITH_SUGGESTION",
  parameters = Map("objectName" -> "`non_exist`", "proposal" -> "`i`, `j`"))

Why are the changes needed?

To follow new naming convention introduced by #44902.

Does this PR introduce any user-facing change?

No, the changes affect only in tests.

How was this patch tested?

By running the existing test suites and GAs.

Was this patch authored or co-authored using generative AI tooling?

No.

@MaxGekk MaxGekk changed the title [WIP][TESTS] Rename errorClass to condition in checkError() [SPARK-49579][SQL][TESTS] Rename errorClass to condition in checkError() Sep 10, 2024
@MaxGekk MaxGekk marked this pull request as ready for review September 10, 2024 06:45
@MaxGekk MaxGekk requested a review from cloud-fan September 10, 2024 06:46
@MaxGekk
Copy link
Member Author

MaxGekk commented Sep 10, 2024

@srielau @panbingkun @nchammas @cloud-fan Could you review the PR, please.

@MaxGekk
Copy link
Member Author

MaxGekk commented Sep 10, 2024

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

@MaxGekk MaxGekk closed this in 0fd91c7 Sep 10, 2024
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Hi, @MaxGekk and @cloud-fan . Unfortunately, this seems to break master branch compilation for some reasons. Could you double-check?

[error] /home/runner/work/spark/spark/core/src/test/scala/org/apache/spark/SparkThrowableSuite.scala:509:5: overloaded method checkError with alternatives:
[error]   (exception: org.apache.spark.SparkThrowable,condition: String,sqlState: Option[String],parameters: Map[String,String],context: SparkThrowableSuite.this.ExpectedContext)Unit <and>
[error]   (exception: org.apache.spark.SparkThrowable,condition: String,sqlState: String,context: SparkThrowableSuite.this.ExpectedContext)Unit <and>
[error]   (exception: org.apache.spark.SparkThrowable,condition: String,parameters: Map[String,String],context: SparkThrowableSuite.this.ExpectedContext)Unit <and>
[error]   (exception: org.apache.spark.SparkThrowable,condition: String,sqlState: String,parameters: Map[String,String],context: SparkThrowableSuite.this.ExpectedContext)Unit <and>
[error]   (exception: org.apache.spark.SparkThrowable,condition: String,sqlState: String,parameters: Map[String,String])Unit <and>
[error]   (exception: org.apache.spark.SparkThrowable,condition: String,sqlState: Option[String],parameters: Map[String,String],matchPVals: Boolean,queryContext: Array[SparkThrowableSuite.this.ExpectedContext])Unit
[error]  [which have no such parameter errorClass] cannot be applied to (exception: org.apache.spark.SparkException, errorClass: String, parameters: scala.collection.immutable.Map[String,String])
[error]     checkError(
[error]     ^

MaxGekk added a commit that referenced this pull request Sep 10, 2024
…Class` in `SparkThrowableSuite` and in `DateTimeFormatterHelperSuite`

### What changes were proposed in this pull request?
In the PR, I propose to use `condition` instead of `errorClass` in two test suites:
- SparkThrowableSuite
- DateTimeFormatterHelperSuite

### Why are the changes needed?
Because the changes from the PR #48027 conflict to #48058 and #48026, and tests in #48027 were passed earlier than the last PRs were merged to master.

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

### How was this patch tested?
By compiling and running the following tests locally:
```
$ build/sbt "test:testOnly *org.apache.spark.sql.catalyst.util.DateTimeFormatterHelperSuite"
$ build/sbt "test:testOnly *SparkThrowableSuite"
```

### Was this patch authored or co-authored using generative AI tooling?
No.

Closes #48061 from MaxGekk/fix-missing-errorClass.

Authored-by: Max Gekk <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
@panbingkun
Copy link
Contributor

late LGTM.

MaxGekk added a commit that referenced this pull request Sep 11, 2024
…Class` in `SqlScriptingInterpreterSuite`

### What changes were proposed in this pull request?
In the PR, I propose to replace `errorClass` by `condition` in `SqlScriptingInterpreterSuite`

### Why are the changes needed?
The changes from the PR #47756 conflict to #48027

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

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

### Was this patch authored or co-authored using generative AI tooling?
No.

Closes #48072 from MaxGekk/fix-errorClass-REPEAT.

Authored-by: Max Gekk <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
vkorukanti pushed a commit to delta-io/delta that referenced this pull request Sep 18, 2024
)

## Description

In the PR, I propose to use the `condition` parameter instead of
`errorClass` in calls of `checkError` because `errorClass` was renamed
in Spark by the PR apache/spark#48027. This PR
fixes compilation issues like:
```
[error]       checkError(
[error]       ^
[error] /home/runner/work/delta/delta/spark/src/test/scala/org/apache/spark/sql/delta/rowtracking/RowTrackingReadWriteSuite.scala:304:7: overloaded method checkError with alternatives:
[error]   (exception: org.apache.spark.SparkThrowable,condition: String,sqlState: Option[String],parameters: Map[String,String],context: RowTrackingReadWriteSuite.this.ExpectedContext)Unit <and>
```

## How was this patch tested?
By compiling locally.

## Does this PR introduce _any_ user-facing changes?
No. This makes changes in tests only.
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.

4 participants