[GitHub] spark pull request #14377: [SPARK-16625][SQL] General data types to be mappe...

[srowen <gi...@git.apache.org>]

Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14377#discussion_r73189845
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/jdbc/OracleDialect.scala ---
    @@ -26,30 +26,38 @@ private case object OracleDialect extends JdbcDialect {
     
       override def canHandle(url: String): Boolean = url.startsWith("jdbc:oracle")
     
    -  override def getCatalystType(
    -      sqlType: Int, typeName: String, size: Int, md: MetadataBuilder): Option[DataType] = {
    +  override def getCatalystType(sqlType: Int, typeName: String, size: Int, md: MetadataBuilder):
    +  Option[DataType] = sqlType match {
         // Handle NUMBER fields that have no precision/scale in special way
         // because JDBC ResultSetMetaData converts this to 0 precision and -127 scale
         // For more details, please see
         // https://github.com/apache/spark/pull/8780#issuecomment-145598968
         // and
         // https://github.com/apache/spark/pull/8780#issuecomment-144541760
    -    if (sqlType == Types.NUMERIC && size == 0) {
    -      // This is sub-optimal as we have to pick a precision/scale in advance whereas the data
    -      //  in Oracle is allowed to have different precision/scale for each value.
    +    case Types.NUMERIC if size == 0 => Option(DecimalType(DecimalType.MAX_PRECISION, 10))
    +    // Handle FLOAT fields in a special way because JDBC ResultSetMetaData converts
    +    // this to NUMERIC with -127 scale
    +    // Not sure if there is a more robust way to identify the field as a float (or other
    +    // numeric types that do not specify a scale.
    +    case Types.NUMERIC if md.build().getLong("scale") == -127 =>
    --- End diff --
    
    @wangyum I can merge this, since it looks reasonable, even though I'm not an Oracle expert. Can we streamline this a bit by only computing `md.build().getLong("scale")` once? The repeated matching of `Types.NUMERIC` could be done once in an `if` statement and be clearer.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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