Skip to content

Conversation

@panbingkun
Copy link
Contributor

@panbingkun panbingkun commented Oct 14, 2022

What changes were proposed in this pull request?

This PR aims to replace 'intercept' with 'Check error classes' in DDLParserSuite. include two files:
A.org.apache.spark.sql.execution.command.DDLParserSuite
B.org.apache.spark.sql.catalyst.parser.DDLParserSuite

Why are the changes needed?

The changes improve the error framework.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

By running the modified test suite:

$ build/sbt "test:testOnly *DDLParserSuite"

@panbingkun
Copy link
Contributor Author

cc @MaxGekk

@github-actions github-actions bot added the SQL label Oct 14, 2022

private def parseException(sqlText: String): SparkThrowable = {
intercept[ParseException](parsePlan(sqlText))
}
Copy link
Contributor

@amaliujia amaliujia Oct 15, 2022

Choose a reason for hiding this comment

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

can this function be shared with other Suites (e.g. #38265) so move this one to AnalysisTest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, i will move it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@MaxGekk
Copy link
Member

MaxGekk commented Oct 15, 2022

@panbingkun Please, don't forget about another DDLParserSuite at:
org.apache.spark.sql.execution.command.DDLParserSuite

@panbingkun
Copy link
Contributor Author

panbingkun commented Oct 15, 2022

@panbingkun Please, don't forget about another DDLParserSuite at: org.apache.spark.sql.execution.command.DDLParserSuite

Done.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@panbingkun
Copy link
Contributor Author

When I work on it, I found typo: #38269
cc @MaxGekk

@panbingkun panbingkun requested a review from amaliujia October 16, 2022 02:51
}

protected def parseException(parser: String => Any)(
sqlText: String): ParseException = {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Let's fix this next time.

@MaxGekk
Copy link
Member

MaxGekk commented Oct 16, 2022

+1, LGTM. Merging to master.
Thank you, @panbingkun.

@MaxGekk MaxGekk closed this in a7ac99b Oct 16, 2022
@amaliujia
Copy link
Contributor

LGTM!

@panbingkun panbingkun deleted the SPARK-40784 branch November 7, 2022 01:59
SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
### What changes were proposed in this pull request?
This PR aims to replace 'intercept' with 'Check error classes' in DDLParserSuite. include two files:
A.org.apache.spark.sql.execution.command.DDLParserSuite
B.org.apache.spark.sql.catalyst.parser.DDLParserSuite

### Why are the changes needed?
The changes improve the error framework.

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

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

Closes apache#38259 from panbingkun/SPARK-40784.

Authored-by: panbingkun <[email protected]>
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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants