This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.0 by this push:
new 3aa500ea30df [SPARK-50666][SQL][FOLLOWUP] Revise JDBC hint support and
remove a typo in comment
3aa500ea30df is described below
commit 3aa500ea30dffcb41db17ad31b021f18b7c7ffd0
Author: beliefer <[email protected]>
AuthorDate: Tue Jan 28 07:05:41 2025 -0800
[SPARK-50666][SQL][FOLLOWUP] Revise JDBC hint support and remove a typo in
comment
### What changes were proposed in this pull request?
This PR proposes to fix some minor issues related to
https://github.com/apache/spark/pull/49564
### Why are the changes needed?
First, simplify some code.
Second, The JDBC dialects doesn't hint should not override the SQL builder.
It's confused.
Third, fix a little incorrect comment.
### Does this PR introduce _any_ user-facing change?
'No'.
New feature.
### How was this patch tested?
GA.
### Was this patch authored or co-authored using generative AI tooling?
'No'.
Closes #49711 from beliefer/SPARK-50666_followup.
Authored-by: beliefer <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 468ae8aa29389ff8428bfd6359aba8653da2aaa9)
Signed-off-by: Dongjoon Hyun <[email protected]>
---
.../spark/sql/execution/datasources/jdbc/JDBCOptions.scala | 10 ++++------
.../src/main/scala/org/apache/spark/sql/jdbc/DB2Dialect.scala | 2 +-
.../scala/org/apache/spark/sql/jdbc/JdbcSQLQueryBuilder.scala | 2 +-
.../scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala | 2 +-
.../src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala | 6 ++----
5 files changed, 9 insertions(+), 13 deletions(-)
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCOptions.scala
b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCOptions.scala
index 9c56ad0433a4..f0c638b7d07c 100644
---
a/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCOptions.scala
+++
b/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCOptions.scala
@@ -249,14 +249,12 @@ class JDBCOptions(
.map(_.toBoolean)
.getOrElse(SQLConf.get.timestampType == TimestampNTZType)
- val hint = {
- parameters.get(JDBC_HINT_STRING).map(value => {
- require(value.matches("(?s)^/\\*\\+ .* \\*/$"),
- s"Invalid value `$value` for option `$JDBC_HINT_STRING`." +
- s" It should start with `/*+ ` and end with ` */`.")
+ val hint = parameters.get(JDBC_HINT_STRING).map(value => {
+ require(value.matches("(?s)^/\\*\\+ .* \\*/$"),
+ s"Invalid value `$value` for option `$JDBC_HINT_STRING`." +
+ s" It should start with `/*+ ` and end with ` */`.")
s"$value "
}).getOrElse("")
- }
override def hashCode: Int = this.parameters.hashCode()
diff --git a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/DB2Dialect.scala
b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/DB2Dialect.scala
index 01028f553324..f33e64c859fb 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/DB2Dialect.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/DB2Dialect.scala
@@ -207,7 +207,7 @@ private case class DB2Dialect() extends JdbcDialect with
SQLConfHelper with NoLe
val offsetClause = dialect.getOffsetClause(offset)
options.prepareQuery +
- s"SELECT $hintClause$columnList FROM ${options.tableOrQuery}
$tableSampleClause" +
+ s"SELECT $columnList FROM ${options.tableOrQuery} $tableSampleClause" +
s" $whereClause $groupByClause $orderByClause $offsetClause
$limitClause"
}
}
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcSQLQueryBuilder.scala
b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcSQLQueryBuilder.scala
index f8b50f496080..95be14f816a7 100644
---
a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcSQLQueryBuilder.scala
+++
b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcSQLQueryBuilder.scala
@@ -69,7 +69,7 @@ class JdbcSQLQueryBuilder(dialect: JdbcDialect, options:
JDBCOptions) {
protected var tableSampleClause: String = ""
/**
- * A hint sample clause representing query hints.
+ * A hint clause representing query hints.
*/
protected val hintClause: String = {
if (options.hint == "" || dialect.supportsHint) {
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala
b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala
index cf943fc6c7c8..77d0891ce338 100644
--- a/sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala
+++ b/sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala
@@ -249,7 +249,7 @@ private case class MsSqlServerDialect() extends JdbcDialect
with NoLegacyJDBCErr
val limitClause = dialect.getLimitClause(limit)
options.prepareQuery +
- s"SELECT $hintClause$limitClause $columnList FROM
${options.tableOrQuery}" +
+ s"SELECT $limitClause $columnList FROM ${options.tableOrQuery}" +
s" $tableSampleClause $whereClause $groupByClause $orderByClause"
}
}
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
index 9a2ccd4785b3..db56da80fd4a 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
@@ -2222,10 +2222,8 @@ class JDBCSuite extends QueryTest with
SharedSparkSession {
val baseParameters = CaseInsensitiveMap(
Map("dbtable" -> "test", "hint" -> "/*+ INDEX(test idx1) */"))
// supported
- Seq(
- "jdbc:oracle:thin:@//host:port",
- "jdbc:mysql://host:port",
- "jdbc:databricks://host:port").foreach { url =>
+ Seq("jdbc:oracle:thin:@", "jdbc:mysql:", "jdbc:databricks:").foreach {
prefix =>
+ val url = s"$prefix//host:port"
val options = new JDBCOptions(baseParameters + ("url" -> url))
val dialect = JdbcDialects.get(url)
assert(dialect.getJdbcSQLQueryBuilder(options)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
