stefanbuk-db commented on code in PR #46642:
URL: https://github.com/apache/spark/pull/46642#discussion_r1606926117


##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -141,6 +160,9 @@ private case class MsSqlServerDialect() extends JdbcDialect 
{
     case ShortType if !SQLConf.get.legacyMsSqlServerNumericMappingEnabled =>
       Some(JdbcType("SMALLINT", java.sql.Types.SMALLINT))
     case ByteType => Some(JdbcType("SMALLINT", java.sql.Types.TINYINT))
+    case LongType => Some(JdbcType("BIGINT", java.sql.Types.BIGINT))
+    case DoubleType => Some(JdbcType("FLOAT", java.sql.Types.FLOAT))
+    case _ if !SQLConf.get.legacyMsSqlServerNumericMappingEnabled => 
JdbcUtils.getCommonJDBCType(dt)

Review Comment:
   If question is about a 
`!SQLConf.get.legacyMsSqlServerNumericMappingEnabled`, it is added here because 
there is this config, false by default, and when it is set, some type mapping 
shouldn't be supported (not sure which, or why there is this config), but if we 
didn't have this check here, some tests with this config would fail, as we 
would, for example convert ShortType to `SMALLINT` even tho we shouldn't.



-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to