cloud-fan commented on code in PR #53902:
URL: https://github.com/apache/spark/pull/53902#discussion_r3499676526
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MySQLDialect.scala:
##########
@@ -36,8 +36,10 @@ import org.apache.spark.sql.types._
private case class MySQLDialect() extends JdbcDialect with SQLConfHelper with
NoLegacyJDBCError {
- override def canHandle(url : String): Boolean =
- url.toLowerCase(Locale.ROOT).startsWith("jdbc:mysql")
+ override def canHandle(url : String): Boolean = {
+ val lower = url.toLowerCase(Locale.ROOT)
+ lower.startsWith("jdbc:mysql") ||
lower.startsWith("jdbc:aws-wrapper:mysql")
Review Comment:
`jdbc:aws-wrapper:mysql` is a single vendor's (AWS
aws-advanced-jdbc-wrapper) library URL scheme, not a JDBC standard, and the
wrapper speaks standard MySQL underneath — so this adds no dialect behavior,
just a hardcoded vendor prefix in core. Rather than enumerate vendor prefixes
here, consider exposing this dialect as an advanced API (drop `private`, add
`@DeveloperApi`) so a user can `class AwsMySQLDialect extends MySQLDialect`,
override `canHandle` for the wrapper prefix, and `JdbcDialects.registerDialect`
it (tried first). Keeps the vendor string out of core while removing the real
friction (the `private` modifier).
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/PostgresDialect.scala:
##########
@@ -40,8 +40,10 @@ import org.apache.spark.sql.types._
private case class PostgresDialect()
extends JdbcDialect with SQLConfHelper with NoLegacyJDBCError {
- override def canHandle(url: String): Boolean =
- url.toLowerCase(Locale.ROOT).startsWith("jdbc:postgresql")
+ override def canHandle(url: String): Boolean = {
+ val lower = url.toLowerCase(Locale.ROOT)
+ lower.startsWith("jdbc:postgresql") ||
lower.startsWith("jdbc:aws-wrapper:postgresql")
Review Comment:
Same concern as MySQLDialect: `jdbc:aws-wrapper:postgresql` is a
vendor-specific prefix, not a JDBC standard. Prefer exposing `PostgresDialect`
as a `@DeveloperApi` advanced API so a wrapper dialect can subclass and
override `canHandle`, instead of hardcoding the prefix in core.
--
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]
