cloud-fan commented on code in PR #56898:
URL: https://github.com/apache/spark/pull/56898#discussion_r3498713808
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala:
##########
@@ -54,6 +54,13 @@ private case class OracleDialect() extends JdbcDialect with
SQLConfHelper with N
e.getMessage.contains("ORA-39165")
}
+ override def isNotSelectableObjectException(e: SQLException): Boolean = {
+ // ORA-04044: object is not a table (e.g. a synonym to a
procedure/function/package).
+ e.getMessage.contains("ORA-04044") ||
Review Comment:
Optional, non-blocking: matching on `e.getMessage.contains(...)` is fragile
to message format/locale; `e.getErrorCode == 4044` (and `4063`) would be more
robust. This mirrors the existing `isObjectNotFoundException` here, so it's
consistent either way — only worth changing if you want to harden both together.
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##########
@@ -808,6 +808,9 @@ abstract class JdbcDialect extends Serializable with
Logging {
Option(e.getSQLState).exists(_.startsWith("42"))
}
+ @Since("4.3.0")
Review Comment:
`4.2.0` is already released (branch-4.2 is on `4.2.1-SNAPSHOT`). A master PR
that backports normally ships first in `branch-4.x` (`4.3.0-SNAPSHOT`), so this
new API should be `@Since("4.3.0")`. (The other `@Since("4.2.0")` annotations
in this file predate the 4.2 cut.)
```suggestion
@Since("4.3.0")
```
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##########
@@ -808,6 +808,9 @@ abstract class JdbcDialect extends Serializable with
Logging {
Option(e.getSQLState).exists(_.startsWith("42"))
}
+ @Since("4.3.0")
+ def isNotSelectableObjectException(e: SQLException): Boolean = false
Review Comment:
This is a general opt-in hook (base default `false`, like
`isObjectNotFoundException`), but only Oracle overrides it today. Consider a
short Scaladoc stating the contract, so the Oracle-only impl reads as the first
adopter and the next dialect author knows when to override. Placed above the
`@Since` line, e.g.:
```scala
/**
* Returns true if the given exception indicates the object exists but
cannot be read as a
* table or view (e.g. a synonym that resolves to a procedure, or an
invalid view), as opposed
* to not existing at all (see `isObjectNotFoundException`). Dialects
override this to recognize
* their own error codes; the default is false.
*/
```
(Left as an illustrative block rather than a one-click suggestion so it
doesn't collide with the `@Since` fix above.)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
