Skip to content

Commit 3aa500e

Browse files
belieferdongjoon-hyun
authored andcommitted
[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 #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 468ae8a) Signed-off-by: Dongjoon Hyun <[email protected]>
1 parent 09876bb commit 3aa500e

File tree

5 files changed

+9
-13
lines changed

5 files changed

+9
-13
lines changed

sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCOptions.scala

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -249,14 +249,12 @@ class JDBCOptions(
249249
.map(_.toBoolean)
250250
.getOrElse(SQLConf.get.timestampType == TimestampNTZType)
251251

252-
val hint = {
253-
parameters.get(JDBC_HINT_STRING).map(value => {
254-
require(value.matches("(?s)^/\\*\\+ .* \\*/$"),
255-
s"Invalid value `$value` for option `$JDBC_HINT_STRING`." +
256-
s" It should start with `/*+ ` and end with ` */`.")
252+
val hint = parameters.get(JDBC_HINT_STRING).map(value => {
253+
require(value.matches("(?s)^/\\*\\+ .* \\*/$"),
254+
s"Invalid value `$value` for option `$JDBC_HINT_STRING`." +
255+
s" It should start with `/*+ ` and end with ` */`.")
257256
s"$value "
258257
}).getOrElse("")
259-
}
260258

261259
override def hashCode: Int = this.parameters.hashCode()
262260

sql/core/src/main/scala/org/apache/spark/sql/jdbc/DB2Dialect.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ private case class DB2Dialect() extends JdbcDialect with SQLConfHelper with NoLe
207207
val offsetClause = dialect.getOffsetClause(offset)
208208

209209
options.prepareQuery +
210-
s"SELECT $hintClause$columnList FROM ${options.tableOrQuery} $tableSampleClause" +
210+
s"SELECT $columnList FROM ${options.tableOrQuery} $tableSampleClause" +
211211
s" $whereClause $groupByClause $orderByClause $offsetClause $limitClause"
212212
}
213213
}

sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcSQLQueryBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ class JdbcSQLQueryBuilder(dialect: JdbcDialect, options: JDBCOptions) {
6969
protected var tableSampleClause: String = ""
7070

7171
/**
72-
* A hint sample clause representing query hints.
72+
* A hint clause representing query hints.
7373
*/
7474
protected val hintClause: String = {
7575
if (options.hint == "" || dialect.supportsHint) {

sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ private case class MsSqlServerDialect() extends JdbcDialect with NoLegacyJDBCErr
249249
val limitClause = dialect.getLimitClause(limit)
250250

251251
options.prepareQuery +
252-
s"SELECT $hintClause$limitClause $columnList FROM ${options.tableOrQuery}" +
252+
s"SELECT $limitClause $columnList FROM ${options.tableOrQuery}" +
253253
s" $tableSampleClause $whereClause $groupByClause $orderByClause"
254254
}
255255
}

sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2222,10 +2222,8 @@ class JDBCSuite extends QueryTest with SharedSparkSession {
22222222
val baseParameters = CaseInsensitiveMap(
22232223
Map("dbtable" -> "test", "hint" -> "/*+ INDEX(test idx1) */"))
22242224
// supported
2225-
Seq(
2226-
"jdbc:oracle:thin:@//host:port",
2227-
"jdbc:mysql://host:port",
2228-
"jdbc:databricks://host:port").foreach { url =>
2225+
Seq("jdbc:oracle:thin:@", "jdbc:mysql:", "jdbc:databricks:").foreach { prefix =>
2226+
val url = s"$prefix//host:port"
22292227
val options = new JDBCOptions(baseParameters + ("url" -> url))
22302228
val dialect = JdbcDialects.get(url)
22312229
assert(dialect.getJdbcSQLQueryBuilder(options)

0 commit comments

Comments
 (0)