Skip to content

[SPARK-46393][SQL][FOLLOWUP] Classify exceptions in JDBCTableCatalog.loadTable and Fix UT #46912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
panbingkun wants to merge 4 commits into from
Closed

[SPARK-46393][SQL][FOLLOWUP] Classify exceptions in JDBCTableCatalog.loadTable and Fix UT #46912

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9f341af
[SPARK-46393][SQL][TESTS] Fix UT
panbingkun Jun 7, 2024
b8a40c0
update
panbingkun Jun 7, 2024
2d136ef
Merge branch 'master' into SPARK-46393_FOLLOWUP
panbingkun Jun 7, 2024
60b4228
[SPARK-46393][SQL][FOLLOWUP] Classify exceptions in JDBCTableCatalog.…
cloud-fan Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
update
  • Loading branch information
@panbingkun
panbingkun committed Jun 7, 2024
commit b8a40c0d0f96dba619d4982cd946802f18af32a9
14 changes: 7 additions & 7 deletions ...tor/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/v2/V2JDBCTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,13 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu

def testCreateTableWithProperty(tbl: String): Unit = {}

def checkErrorFailedLoadTable(e: AnalysisException): Unit = {
def checkErrorFailedLoadTable(e: AnalysisException, tbl: String): Unit = {
checkError(
exception = e,
errorClass = "FAILED_JDBC.UNCLASSIFIED",
Copy link
Contributor

@cloud-fan cloud-fan Jun 7, 2024

Choose a reason for hiding this comment

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

shouldn't this be LOAD_TABLE?

Copy link
Contributor Author

@panbingkun panbingkun Jun 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

spark/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala

Lines 746 to 768 in 87b0f59

def classifyException(
e: Throwable,
errorClass: String,
messageParameters: Map[String, String],
description: String): AnalysisException = {
classifyException(description, e)
}
/**
* Gets a dialect exception, classifies it and wraps it by `AnalysisException`.
* @param message The error message to be placed to the returned exception.
* @param e The dialect specific exception.
* @return `AnalysisException` or its sub-class.
*/
@deprecated("Please override the classifyException method with an error class", "4.0.0")
def classifyException(message: String, e: Throwable): AnalysisException = {
new AnalysisException(
errorClass = "FAILED_JDBC.UNCLASSIFIED",
messageParameters = Map(
"url" -> "jdbc:",
"message" -> message),
cause = Some(e))
}

image
It lost the upstream errorClass in this place

Copy link
Contributor Author

@panbingkun panbingkun Jun 7, 2024

Choose a reason for hiding this comment

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

It seems that we should correct it.

Copy link
Contributor Author

@panbingkun panbingkun Jun 7, 2024

Choose a reason for hiding this comment

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

If necessary, I handle it together with the prefix logic above?

Copy link
Contributor

@cloud-fan cloud-fan Jun 7, 2024

Choose a reason for hiding this comment

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

SGTM

parameters = Map(
"url" -> "jdbc:",
"message" -> "Failed to load table: not_existing_table"
"message" -> s"Failed to load table: $tbl"
)
)
}
Expand Down Expand Up @@ -132,7 +132,7 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
val e = intercept[AnalysisException] {
sql(s"ALTER TABLE $catalogName.not_existing_table ADD COLUMNS (C4 STRING)")
}
checkErrorFailedLoadTable(e)
checkErrorFailedLoadTable(e, "not_existing_table")
}

test("SPARK-33034: ALTER TABLE ... drop column") {
Expand All @@ -154,7 +154,7 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
val e = intercept[AnalysisException] {
sql(s"ALTER TABLE $catalogName.not_existing_table DROP COLUMN C1")
}
checkErrorFailedLoadTable(e)
checkErrorFailedLoadTable(e, "not_existing_table")
}

test("SPARK-33034: ALTER TABLE ... update column type") {
Expand All @@ -170,7 +170,7 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
val e = intercept[AnalysisException] {
sql(s"ALTER TABLE $catalogName.not_existing_table ALTER COLUMN id TYPE DOUBLE")
}
checkErrorFailedLoadTable(e)
checkErrorFailedLoadTable(e, "not_existing_table")
}

test("SPARK-33034: ALTER TABLE ... rename column") {
Expand Down Expand Up @@ -198,7 +198,7 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
val e = intercept[AnalysisException] {
sql(s"ALTER TABLE $catalogName.not_existing_table RENAME COLUMN ID TO C")
}
checkErrorFailedLoadTable(e)
checkErrorFailedLoadTable(e, "not_existing_table")
}

test("SPARK-33034: ALTER TABLE ... update column nullability") {
Expand All @@ -209,7 +209,7 @@ private[v2] trait V2JDBCTest extends SharedSparkSession with DockerIntegrationFu
val e = intercept[AnalysisException] {
sql(s"ALTER TABLE $catalogName.not_existing_table ALTER COLUMN ID DROP NOT NULL")
}
checkErrorFailedLoadTable(e)
checkErrorFailedLoadTable(e, "not_existing_table")
}

test("CREATE TABLE with table comment") {
Expand Down