mattyb149 commented on a change in pull request #3302: NIFI-6000 Catch also
IllegalArgumentException in ConvertAvroToORC hiv…
URL: https://github.com/apache/nifi/pull/3302#discussion_r256146321
##########
File path:
nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/main/java/org/apache/nifi/processors/hive/ConvertAvroToORC.java
##########
@@ -283,8 +283,8 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
session.transfer(flowFile, REL_SUCCESS);
session.getProvenanceReporter().modifyContent(flowFile, "Converted
"+totalRecordCount.get()+" records", System.currentTimeMillis() - startTime);
- } catch (final ProcessException pe) {
- getLogger().error("Failed to convert {} from Avro to ORC due to
{}; transferring to failure", new Object[]{flowFile, pe});
+ } catch (ProcessException | IllegalArgumentException e) {
+ getLogger().error("Failed to convert {} from Avro to ORC due to
{}; transferring to failure", new Object[]{flowFile, e});
Review comment:
Since the ORC Utils can throw an IllegalArgumentException, you are right
that we should catch it here for more robust handling. However, since we know
the field must contain nulls or a (possibly empty) array of nulls, why not
support them by creating an ORC (and Hive DDL) column of some default type,
such as `boolean` (since it's the smallest)? Then instead of routing to
failure, it will actually be successful and you can even create a Hive table
atop it.
I took the liberty of building off your PR and adding support for the Avro
null type using the aforementioned approach. I updated the unit tests to go to
success (and removed the Avro content validation since it has been successfully
converted to ORC) and added one that still exercises your logic, namely that we
don't currently support the Avro `fixed` type either. We should add support for
that as well, but that can be done as a separate Jira so we can get this logic
into the codebase.
If you're interested in looking at and/or incorporating my commit into your
PR, it is here
(https://github.com/mattyb149/nifi/commit/5ca30de053ecb90845f5c2c64fa96f2a79edd393).
I haven't run it all the way to see if the Hive table works, but since the
column exists in the file and the table, it seems like it should work fine.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
