[spark] branch master updated: [SPARK-42469][SQL] Update MSSQL Dialect to use parentheses for TOP and add tests for Limit clause

[dongjoon]

This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new 9d876844082 [SPARK-42469][SQL] Update MSSQL Dialect to use parentheses 
for TOP and add tests for Limit clause
9d876844082 is described below

commit 9d876844082d588a15e09b20f2cd229041ea556b
Author: Ivan Sadikov <ivan.sadi...@databricks.com>
AuthorDate: Thu Feb 16 20:38:53 2023 -0800

    [SPARK-42469][SQL] Update MSSQL Dialect to use parentheses for TOP and add 
tests for Limit clause
    
    ### What changes were proposed in this pull request?
    
    This PR is a follow-up for 
https://issues.apache.org/jira/browse/SPARK-42131. It adds parentheses in TOP 
clause in MSSQL dialect as they are only omitted for backward compatibility and 
required otherwise: 
https://learn.microsoft.com/en-us/sql/t-sql/queries/top-transact-sql?view=sql-server-ver16#compatibility-support.
    
    I also added tests to check Limit clause translation for Oracle and MSSQL 
dialects.
    
    ### Why are the changes needed?
    
    Updates TOP to include round brackets.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    I added unit tests.
    
    Closes #40059 from sadikovi/jdbc-small-fix.
    
    Authored-by: Ivan Sadikov <ivan.sadi...@databricks.com>
    Signed-off-by: Dongjoon Hyun <dongj...@apache.org>
---
 .../apache/spark/sql/jdbc/MsSqlServerDialect.scala |  2 +-
 .../org/apache/spark/sql/jdbc/JDBCSuite.scala      | 28 +++++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

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 39b617135ce..156f495943e 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
@@ -183,7 +183,7 @@ private object MsSqlServerDialect extends JdbcDialect {
   }
 
   override def getLimitClause(limit: Integer): String = {
-    if (limit > 0) s"TOP $limit" else ""
+    if (limit > 0) s"TOP ($limit)" else ""
   }
 
   override def classifyException(message: String, e: Throwable): 
AnalysisException = {
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 aa66fcd5304..27609de5433 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
@@ -959,6 +959,19 @@ class JDBCSuite extends QueryTest with SharedSparkSession {
       Some(TimestampType))
   }
 
+  test("SPARK-42469: OracleDialect Limit query test") {
+    // JDBC url is a required option but is not used in this test.
+    val options = new JDBCOptions(Map("url" -> "jdbc:h2://host:port", 
"dbtable" -> "test"))
+    assert(
+      OracleDialect
+        .getJdbcSQLQueryBuilder(options)
+        .withColumns(Array("a", "b"))
+        .withLimit(123)
+        .build()
+        .trim() ==
+      "SELECT tab.* FROM (SELECT a,b FROM test    ) tab WHERE rownum <= 123")
+  }
+
   test("MsSqlServerDialect jdbc type mapping") {
     val msSqlServerDialect = JdbcDialects.get("jdbc:sqlserver")
     
assert(msSqlServerDialect.getJDBCType(TimestampType).map(_.databaseTypeDefinition).get
 ==
@@ -981,7 +994,7 @@ class JDBCSuite extends QueryTest with SharedSparkSession {
     }
   }
 
-  test("SPARK-28152 MsSqlServerDialect catalyst type mapping") {
+  test("SPARK-28152: MsSqlServerDialect catalyst type mapping") {
     val msSqlServerDialect = JdbcDialects.get("jdbc:sqlserver")
     val metadata = new MetadataBuilder().putLong("scale", 1)
 
@@ -1002,6 +1015,19 @@ class JDBCSuite extends QueryTest with 
SharedSparkSession {
     }
   }
 
+  test("SPARK-42469: MsSqlServerDialect Limit query test") {
+    // JDBC url is a required option but is not used in this test.
+    val options = new JDBCOptions(Map("url" -> "jdbc:h2://host:port", 
"dbtable" -> "test"))
+    assert(
+      MsSqlServerDialect
+        .getJdbcSQLQueryBuilder(options)
+        .withColumns(Array("a", "b"))
+        .withLimit(123)
+        .build()
+        .trim() ==
+      "SELECT TOP (123) a,b FROM test")
+  }
+
   test("table exists query by jdbc dialect") {
     val MySQL = JdbcDialects.get("jdbc:mysql://127.0.0.1/db")
     val Postgres = JdbcDialects.get("jdbc:postgresql://127.0.0.1/db")


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org