You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org> on 2026/09/30 16:37:35 UTC
[PR] [SPARK-59891][SQL] Classify only syntax error codes as syntax errors in Teradata dialect [spark]
SEPURI-SAI-KRISHNA opened a new pull request, #59160:
URL: https://github.com/apache/spark/pull/59160
### What changes were proposed in this pull request?
This PR makes `TeradataDialect.isSyntaxErrorBestEffort` match the Teradata error codes for syntax errors (3706, 3707, 3708 and 3709, the ones whose message starts with "Syntax error") instead of every SQLSTATE of class 42.
This is a follow-up of SPARK-59336, which made the same change for PostgreSQL, and covers the Teradata part of [this review comment](https://github.com/apache/spark/pull/58621#discussion_r3968178897) on it.
### Why are the changes needed?
`JdbcDialect.isSyntaxErrorBestEffort` promises that a `true` result is guaranteed to be a syntax error. The Teradata JDBC driver sets the SQLSTATE from a fixed error code mapping inside the driver, and with that mapping:
| Error code | Message | SQLSTATE |
|---|---|---|
| 3706 to 3709 | `Syntax error: ...`, `Syntax error, expected something like ...`, ... | `42000` |
| 3523 | `... does not have SELECT access to ...` | `42000` |
| 3524 | `... does not have ... access to database ...` | `42000` |
| 3807 | `Object '...' does not exist.` | `42S02` |
| 3810 | `Column/Parameter '...' does not exist.` | `42S22` |
All of these were wrapped as `JDBC_EXTERNAL_ENGINE_SYNTAX_ERROR`, which hides what actually went wrong. Because syntax errors and access errors share `42000`, the SQLSTATE cannot be used to tell them apart, so this PR matches error codes instead, like `isObjectNotFoundException` in the same dialect already does (`3807`).
### Does this PR introduce _any_ user-facing change?
Yes. With Teradata, errors that are not syntax errors, such as a missing table, a missing column or a missing privilege, are no longer reported as `JDBC_EXTERNAL_ENGINE_SYNTAX_ERROR`; the original driver exception is thrown instead. Syntax errors are reported as before.
### How was this patch tested?
- New unit test in `JDBCSuite` for the syntax and non-syntax error codes above. It fails without this change.
- Checked the mapping in the Teradata JDBC driver itself: SQLSTATEs are computed in the driver from the error code, so I dumped the class 42 part of the mapping for every error code. It is the same in `terajdbc` 16.20.00.13, 17.20.00.12, 20.00.00.46 and 20.00.00.60 (the latest). I also built the exceptions through the driver's own factory for the codes above to confirm the error code and SQLSTATE. I did not run against a Teradata server.
- Checked the message texts in the [Teradata database messages reference](https://docs.teradata.com/r/VMware/Analytics-Database-Messages).
- Ran `JDBCSuite`.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5.5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59891][SQL] Classify only syntax error codes as syntax errors in Teradata dialect [spark]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on code in PR #59160:
URL: https://github.com/apache/spark/pull/59160#discussion_r4218269037
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala:
##########
@@ -2575,6 +2575,29 @@ class JDBCSuite extends SharedSparkSession {
specifiedPrecision, null) == Some(DecimalType.SYSTEM_DEFAULT))
}
+ test("SPARK-59891: TeradataDialect classifies only syntax error codes as syntax errors") {
Review Comment:
Thanks for taking a look. There's no Teradata docker image we could use: Vantage Express only ships as a VM image behind a Teradata account login, which is also why docker-integration-tests has no Teradata suite.
Agreed that the test was mostly restating the logic. One thing that helps here: the Teradata server doesn't send a SQLSTATE at all, the JDBC driver derives it on the client from the error code. `terajdbc` is already a test dependency of sql/core, so I changed the test to build the exceptions with the driver's own `ErrorFactory.makeDatabaseSQLException`, which is what the driver uses for errors returned by the database, instead of hand-written `SQLException`s. It now checks the dialect against what the driver actually throws, including that access errors like 3523/3524 come with the same `42000` as syntax errors. With the old `startsWith("42")` check it fails on 3523.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59891][SQL] Classify only syntax error codes as syntax errors in Teradata dialect [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #59160:
URL: https://github.com/apache/spark/pull/59160#issuecomment-5922298434
cc @srielau FYI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59891][SQL] Classify only syntax error codes as syntax errors in Teradata dialect [spark]
Posted by "urosstan-db (via GitHub)" <gi...@apache.org>.
urosstan-db commented on code in PR #59160:
URL: https://github.com/apache/spark/pull/59160#discussion_r4210168481
##########
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala:
##########
@@ -2575,6 +2575,29 @@ class JDBCSuite extends SharedSparkSession {
specifiedPrecision, null) == Some(DecimalType.SYSTEM_DEFAULT))
}
+ test("SPARK-59891: TeradataDialect classifies only syntax error codes as syntax errors") {
Review Comment:
Do we have tereadata docker image to test new behaviour, since this unit test has almost no value since it is just a copy of simple one-line prod logic without real errors
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59891][SQL] Classify only syntax error codes as syntax errors in Teradata dialect [spark]
Posted by "SEPURI-SAI-KRISHNA (via GitHub)" <gi...@apache.org>.
SEPURI-SAI-KRISHNA commented on PR #59160:
URL: https://github.com/apache/spark/pull/59160#issuecomment-5915547009
@uros-b, this covers the Teradata part of your comment on #58621.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org