[SPARK-49730][SQL] classify syntax errors for pgsql, mysql, sqlserver and h2 #48368

ivanjevtic-db · 2024-10-07T12:55:25Z

What changes were proposed in this pull request?

In this PR I propose that we add classification of JDBC driver exception that represent syntax errors on external databases.
Queries generated from Spark that are sent to connectors should never have syntax error, and always indicate to a bug in code.

Having this classification would greatly help in identifying bugs faster and improving quality.

Documentation about error codes for syntax errors:
https://learn.microsoft.com/en-us/sql/relational-databases/errors-events/database-engine-events-and-errors-0-to-999?view=sql-server-ver16
https://www.postgresql.org/docs/current/errcodes-appendix.html
https://mariadb.com/kb/en/mariadb-error-code-reference/
https://www.h2database.com/javadoc/org/h2/api/ErrorCode.html

Why are the changes needed?

This gives customer more precise error message about issue being hit.
Help identify bugs faster as syntax error thrown from jdbc connectors should never happen.

Does this PR introduce any user-facing change?

Yes, previously customers would get FAILED_JDBC. UNCLASSIFIED or internal error when syntax error occurs in JDBC, whereas they will now get FAILED_JDBC.SYNTAX_ERROR

How was this patch tested?

Integration tests

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

No

…o syntaxErrorsJdbc

…ax-errors-jdbc

jovanpavl-db · 2024-10-07T13:22:18Z

What about internal errors on custom connector side? Can we catch and classify those exceptions (based on the stack trace). For the example, we could have case where there is a miss match between database's schema and dataframe's schema, we wouldn't be able to detect this?

jovanpavl-db · 2024-10-07T13:35:55Z

...integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala

    assert(types(1).equals("class java.lang.String"))
  }

+  test("SPARK-49730: syntax error classification") {


What about tests for GET_SCHEMA and EXECUTE_QUERY?

MaxGekk · 2024-10-07T17:44:07Z

common/utils/src/main/resources/error/error-conditions.json

+      },
+      "SYNTAX_ERROR" : {
+        "message" : [
+          "Query generated for external DBMS, during compile contains syntax error. Original query: <query>."


See the entire error message with parent's error message:

Suggested change

"Query generated for external DBMS, during compile contains syntax error. Original query: <query>."

"Compilation of the query for an external DBMS: <query>."

MaxGekk · 2024-10-07T17:44:39Z

common/utils/src/main/resources/error/error-conditions.json

+      },
+      "EXECUTE_QUERY" : {
+        "message" : [
+          "Failed to execute jdbc query: <query>."


Suggested change

"Failed to execute jdbc query: <query>."

"Execution of the query: <query>."

MaxGekk · 2024-10-07T17:45:38Z

common/utils/src/main/resources/error/error-conditions.json

      },
+      "GET_SCHEMA" : {
+        "message" : [
+          "Fetching schema for external query or table has failed."


Suggested change

"Fetching schema for external query or table has failed."

"Schema fetching for an external query or a table."

ivanjevtic-db · 2024-10-08T12:21:48Z

What about internal errors on custom connector side? Can we catch and classify those exceptions (based on the stack trace). For the example, we could have case where there is a miss match between database's schema and dataframe's schema, we wouldn't be able to detect this?

Does this pr solve it? @jovanpavl-db

ivanjevtic-db · 2024-10-20T11:13:03Z

What should be EXECUTE_QUERY errors?

ivanjevtic-db · 2024-10-20T11:16:22Z

I moved CREATE TABLE with table property test from V2JDBCTest to each dialect test suite. The tests reports syntax error for mssql: com.microsoft.sqlserver.jdbc.SQLServerException: Incorrect syntax near 'a'.

ivanjevtic-db · 2024-10-20T11:17:13Z

Moved tests to V2 Suites.

…nto syntax-errors-jdbc

ivanjevtic-db · 2024-10-23T09:49:25Z

core/src/test/scala/org/apache/spark/SparkFunSuite.scala

          val parm = parameters.getOrElse(exp._1,
            throw new IllegalArgumentException("Missing parameter" + exp._1))
-          if (!exp._2.matches(parm)) {
+          if (!exp._2.matches(parm) && exp._2 != parm) {


Since I am using checkErrorMatchPVals function, either I should:

have this change so that we check regex or check if the exact value is the same or:

from a variable which is the exact match to expected, create something like this: "parameter" -> ("^" + Pattern.quote(val) + "$").

I am using checkErrorMatchPVals in the first place, since I have some parameters that can only be checked with regex, but for some parameters I have the exact value.

ivanjevtic-db · 2024-10-23T13:27:52Z

@MaxGekk I am ready for a review.

MaxGekk

@ivanjevtic-db Could you fix the failed tests like:

[info] - Error conditions are correctly formatted *** FAILED *** (112 milliseconds)
[info]   "...  ]
[info]         },
[info]         "[EXECUTE_QUERY" : {
[info]           "message" : [
[info]             "Execution of the query: <query>."
[info]           ]
[info]         },

ivanjevtic-db · 2024-10-30T16:16:11Z

jenkins trigger all

MaxGekk · 2024-10-30T16:37:28Z

jenkins trigger all

@ivanjevtic-db Don't think it works here.

ivanjevtic-db · 2024-11-01T11:59:30Z

@ivanjevtic-db Could you fix the failed tests like:

[info] - Error conditions are correctly formatted *** FAILED *** (112 milliseconds)
[info]   "...  ]
[info]         },
[info]         "[EXECUTE_QUERY" : {
[info]           "message" : [
[info]             "Execution of the query: <query>."
[info]           ]
[info]         },

Fixed

github-actions · 2025-02-10T00:25:23Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

milastdbx and others added 14 commits September 20, 2024 10:16

classify syntax errors for pgsql, mysql and sqlserver

3f36b20

Merge branch 'apache:master' into syntaxErrorsJdbc

35100c2

Add isRuntime arg to classifyException signature

a6a9f93

Merge branch 'syntaxErrorsJdbc' of github.com:ivanjevtic-db/spark int…

2b8926d

…o syntaxErrorsJdbc

Merge branch 'apache:master' into syntaxErrorsJdbc

a31f363

Finish revert

e205788

Merge branch 'syntaxErrorsJdbc' of github.com:ivanjevtic-db/spark int…

1d54e5a

…o syntaxErrorsJdbc

Remove unused import

bd1fb3f

Remove deprecated function from subclass

c68ab5b

Remove unused import

a65b3c4

Fix comments

17632b1

Fix wrong override

977792c

Merge branch 'Change-classify-exception-function-signature' into synt…

935d8f3

…ax-errors-jdbc

Classify syntax errors for pgsql, mysql, sqlserver

685c212

github-actions bot added the SQL label Oct 7, 2024

Resolve conflicts

21734fd

jovanpavl-db reviewed Oct 7, 2024

View reviewed changes

MaxGekk requested changes Oct 7, 2024

View reviewed changes

ivanjevtic-db added 4 commits October 15, 2024 15:46

Fix error-conditions messages

0793158

Merge remote-tracking branch 'origin/master' into syntax-errors-jdbc

5260e92

Fix syntax and schema errors, Add tests

6ec9aa3

Merge branch 'master' into syntax-errors-jdbc

894fab6

ivanjevtic-db added 3 commits October 20, 2024 13:22

Remove unused import

19cbc07

Merge branch 'syntax-errors-jdbc' of github.com:ivanjevtic-db/spark i…

c587b07

…nto syntax-errors-jdbc

Remove unused import

1edcb97

ivanjevtic-db added 4 commits October 21, 2024 11:32

Fix parameters in tests

0db4dc6

Use checkError with matchPVals=true

387563f

Match parameters to query

737d1db

Fix checkError to check for value too

8b9ef47

github-actions bot added the CORE label Oct 23, 2024

ivanjevtic-db commented Oct 23, 2024

View reviewed changes

Remove commented tests

96c59ac

MaxGekk reviewed Oct 30, 2024

View reviewed changes

ivanjevtic-db added 5 commits October 31, 2024 11:10

Fix

d3d224f

Remove failed test

94b64da

Fix error-conditions.json

c848332

Sort by name

1c19246

Remove sync changes from error-conditions.json

170e194

github-actions bot added the Stale label Feb 10, 2025

github-actions bot closed this Feb 11, 2025

	"Query generated for external DBMS, during compile contains syntax error. Original query: <query>."
	"Compilation of the query for an external DBMS: <query>."

	"Failed to execute jdbc query: <query>."
	"Execution of the query: <query>."

	"Fetching schema for external query or table has failed."
	"Schema fetching for an external query or a table."

[SPARK-49730][SQL] classify syntax errors for pgsql, mysql, sqlserver and h2 #48368

[SPARK-49730][SQL] classify syntax errors for pgsql, mysql, sqlserver and h2 #48368

Uh oh!

Conversation

ivanjevtic-db commented Oct 7, 2024

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

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

Uh oh!

jovanpavl-db commented Oct 7, 2024

Uh oh!

jovanpavl-db Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

MaxGekk Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

MaxGekk Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

MaxGekk Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

ivanjevtic-db commented Oct 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivanjevtic-db commented Oct 20, 2024

Uh oh!

ivanjevtic-db commented Oct 20, 2024

Uh oh!

ivanjevtic-db commented Oct 20, 2024

Uh oh!

ivanjevtic-db Oct 23, 2024

Choose a reason for hiding this comment

Uh oh!

ivanjevtic-db commented Oct 23, 2024

Uh oh!

MaxGekk left a comment

Choose a reason for hiding this comment

Uh oh!

ivanjevtic-db commented Oct 30, 2024

Uh oh!

MaxGekk commented Oct 30, 2024

Uh oh!

ivanjevtic-db commented Nov 1, 2024

Uh oh!

github-actions bot commented Feb 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ivanjevtic-db commented Oct 8, 2024 •

edited

Loading