[GitHub] [pulsar] lynnmatrix commented on a change in pull request #6622: ISSUE-6612 FIX: parse long field in GenricJsonRecord (#6612)

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

lynnmatrix commented on a change in pull request #6622: ISSUE-6612 FIX: parse long field in GenricJsonRecord (#6612)
URL: https://github.com/apache/pulsar/pull/6622#discussion_r401031555
 
 

 ##########
 File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/generic/GenericJsonRecord.java
 ##########
 @@ -55,12 +55,8 @@ public Object getField(String fieldName) {
             return new GenericJsonRecord(schemaVersion, fields, fn);
         } else if (fn.isBoolean()) {
             return fn.asBoolean();
-        } else if (fn.isInt()) {
-            return fn.asInt();
-        } else if (fn.isFloatingPointNumber()) {
-            return fn.asDouble();
-        } else if (fn.isDouble()) {
-            return fn.asDouble();
+        } else if (fn.isNumber()) {
 
 Review comment:
   if fn is long, then fn.aslong() should equals to fn.numberValue()。
   The differences compared to the previous code are:
   1.  If fn is BigDecimal, the new code will return BigDecimal rather than double
   2. If fn is BigInteger, the new code will return BigInteger rather than text
   3. If fn is float, the new code will return float rather than double
   
   So i modify the code like:
   ``` java
           } else if (fn.isBoolean()) {
               return fn.asBoolean();
           } else if (fn.isFloatingPointNumber()) {
               return fn.asDouble();
           } else if (fn.isBigInteger()) {
               if (fn.canConvertToLong()) {
                   return fn.asLong();
               } else {
                   return fn.asText();
               }
           } else if (fn.isNumber()) {
               return fn.numberValue();
           } else {
               return fn.asText();
           }
   ```
   I am trying to do integration tests locally but it takes too long to download snapshot jars from maven. I'll try again, and if fail again, can I push directly and do ci test?

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