[GitHub] [incubator-livy] andrasbeni commented on a change in pull request #288: [LIVY-754][THRIFT] Encode precision and scale for decimal type.

[GitBox <gi...@apache.org>]

andrasbeni commented on a change in pull request #288: [LIVY-754][THRIFT] Encode precision and scale for decimal type.
URL: https://github.com/apache/incubator-livy/pull/288#discussion_r402130614
 
 

 ##########
 File path: thriftserver/server/src/main/scala/org/apache/livy/thriftserver/types/Schema.scala
 ##########
 @@ -109,9 +109,42 @@ object Schema {
       case _ => TTypeId.STRING_TYPE
     }
     val primitiveEntry = new TPrimitiveTypeEntry(typeId)
+    if (dt == DataType.DECIMAL) {
+      val qualifiers = getDecimalQualifiers(name)
+      primitiveEntry.setTypeQualifiers(qualifiers)
+    }
     val entry = TTypeEntry.primitiveEntry(primitiveEntry)
     val desc = new TTypeDesc
     desc.addToTypes(entry)
     desc
   }
+
+  private def getDecimalQualifiers(name: String): TTypeQualifiers = {
+    // name can be one of
+    // 1. decimal
+    // 2. decimal(p)
+    // 3. decimal(p, s)
 
 Review comment:
   Are `decimal` and `decimal(p)` actually possible? I understand these forms can be used to declare the type but based on org.apache.spark.sql.types.DecimalType I don't think the json omits scale or precision.
   
   I might be wrong here. If so, then I believe the parsing logic should be tested for `decimal` and `decimal(p)` also. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services