cloud-fan commented on code in PR #59503:
URL: https://github.com/apache/spark/pull/59503#discussion_r4284910039


##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -166,6 +166,7 @@ private case class MsSqlServerDialect() extends JdbcDialect 
with NoLegacyJDBCErr
       case java.sql.Types.REAL if 
!SQLConf.get.legacyMsSqlServerNumericMappingEnabled =>
         Some(FloatType)
       case GEOMETRY | GEOGRAPHY => Some(BinaryType)
+      case JSON => Some(StringType)

Review Comment:
   **Nit (P3):** Could you add a `json` -> `StringType` row to the "Mapping 
Spark SQL Data Types from Microsoft SQL Server" table in 
`docs/sql-data-sources-jdbc.md`? Every other special case in this 
`getCatalystType` (datetimeoffset, tinyint/smallint, real, geography/geometry) 
has a row there. Without one, json would be the only special-cased SQL Server 
read mapping with no doc row, and the docs would not tell users that native 
json columns are now readable or that they come back as unparsed strings. It's 
a single row, which could go right after `geometry`.
   
   **Verification:**
   - **Inspection:** The Microsoft SQL Server to Spark SQL read-mapping table 
contains exactly one json entry mapped to StringType, and its row structure 
(three cells per row) matches the neighboring rows.



-- 
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]

Reply via email to