[GitHub] [nifi] ijokarumawak commented on issue #3353: NIFI-6105: Fix handling of arrays of records/maps in record utilities

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

ijokarumawak commented on issue #3353: NIFI-6105: Fix handling of arrays of records/maps in record utilities
URL: https://github.com/apache/nifi/pull/3353#issuecomment-470434702
 
 
   @mattyb149 Thanks for the fix! I reviewed and tested the PR, and wanted to discuss about the change a bit.
   
   As a summary, here are the key points:
   1. The reported issue at StackOverflow has been addressed by other changes since NiFI 1.9.0.
   2. But the same flow doesn't work well with schema inference, conversion result has `null` elements.
   3. To fix no.2 (I assume), this PR modifies DataTypeUtils.isCompatibleDataType. However, I suggest fixing DataTypeUtils.toRecord method instead. DataTypeUtils.toRecord converts Java Map to MapRecord before isCompatibleDataType is called, but it doesn't do so recursively. https://github.com/apache/nifi/blob/master/nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/util/DataTypeUtils.java#L302
   
   I may be wrong about above analysis.. How do you think?
   
   Please refer followings for details.
   
   ---
   Details:
   
   I've tested the Stackoverflow questions flow with NiFi 1.8.0 and I got the reported exception:
   ```
   2019-03-07 16:51:40,061 ERROR [Timer-Driven Process Thread-6] o.a.n.p.jolt.record.JoltTransformRecord JoltTransformRecord[id=72a28625-3c23-3bc6-73cd-c06d247bf8dc] Unable to write transformed records StandardFlowFileRecord[uuid=c8b59dad-ff78-4c49-898b-9ac6923eb94c,claim=StandardContentClaim [resourceClaim=StandardResourceClaim[id=1551945098069-1, container=default, section=1], offset=0, length=873],offset=0,name=c8b59dad-ff78-4c49-898b-9ac6923eb94c,size=873] due to org.apache.nifi.serialization.record.util.IllegalTypeConversionException: Cannot convert value of class [Ljava.lang.Object; because the type is not supported: org.apache.nifi.serialization.record.util.IllegalTypeConversionException: Cannot convert value of class [Ljava.lang.Object; because the type is not supported
   org.apache.nifi.serialization.record.util.IllegalTypeConversionException: Cannot convert value of class [Ljava.lang.Object; because the type is not supported
           at org.apache.nifi.serialization.record.util.DataTypeUtils.convertRecordFieldtoObject(DataTypeUtils.java:460)
           at org.apache.nifi.serialization.record.util.DataTypeUtils.convertRecordMapToJavaMap(DataTypeUtils.java:472)
           at org.apache.nifi.serialization.record.util.DataTypeUtils.convertRecordFieldtoObject(DataTypeUtils.java:444)
           at org.apache.nifi.processors.jolt.record.JoltTransformRecord.onTrigger(JoltTransformRecord.java:318)
   ```
   
   However, the flow works fine with NiFi 1.9.0 which has been released recently.
   So, the reported issue at Stackoverflow has been fixed by different commit.
   
   Further testing, I found that with NiFi 1.10.0 snapshot master branch, if I used schema inference, result becomes null like this:
   Before applying this fix, conversion result was:
   ```
   [{"DataItems":[null,null,null,null]}]
   ```
   This happened because the output data type was not found for the inferred Choice[Record, Record] was not compatible with the Java Map type. I assume that's why you fixed isCompatibleDataType method.
   https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-standard-services/nifi-record-serialization-services-bundle/nifi-record-serialization-services/src/main/java/org/apache/nifi/json/WriteJsonResult.java#L284
   
   
   Although I got the expected result with this fix even with schema inference, I think it's more appropriate to fix DataTypeUtils.toRecord method, instead of isCompatibleDataType.
   Specifically, following linked line should convert java Map type to MapRecord recursively. Then isCompatibleDataType didn't have to be modified.
   https://github.com/apache/nifi/blob/master/nifi-commons/nifi-record/src/main/java/org/apache/nifi/serialization/record/util/DataTypeUtils.java#L302

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