[GitHub] [nifi] tpalfy commented on a change in pull request #4223: NIFI-7369 Adding big decimal support for record handling in order to avoid missing precision when reading in records

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

tpalfy commented on a change in pull request #4223:
URL: https://github.com/apache/nifi/pull/4223#discussion_r422237710



##########
File path: nifi-nar-bundles/nifi-extension-utils/nifi-record-utils/nifi-avro-record-utils/src/main/java/org/apache/nifi/avro/AvroTypeUtil.java
##########
@@ -256,6 +256,12 @@ private static Schema buildAvroSchema(final DataType dataType, final String fiel
             case LONG:
                 schema = Schema.create(Type.LONG);
                 break;
+            case BIGDECIMAL:
+                // One more byte than below to allow the dot in the string representation
+                schema = Schema.createFixed(fieldName + "Type", null,  "org.apache.nifi",39);

Review comment:
       Not sure where the concept that this should be a fixed sized type (with namespace) comes from.
   Also where did the 38 maximum come from? I remember seeing such a max in ORC, but not sure about Avro.

##########
File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive3-processors/src/main/java/org/apache/hadoop/hive/ql/io/orc/NiFiOrcUtils.java
##########
@@ -284,6 +290,11 @@ public static TypeInfo getOrcField(DataType dataType, boolean hiveFieldNames) th
                 || RecordFieldType.STRING.equals(fieldType)) {
             return getPrimitiveOrcTypeFromPrimitiveFieldType(dataType);
         }
+
+        if (RecordFieldType.BIGDECIMAL.equals(fieldType)) {
+            // 38 is the maximum allowed precision and 19 digit is needed to represent long values

Review comment:
       Not sure I understand these constraints. With this, no bigger than 19 digit numbers (apart from decimal part) can be used.
   All-in-all I'm wondering if the `RecordFieldType.BIGDECIMAL` can stay as it is, without being able to convey precision and scale information.




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