GH-858: Fix error handling in CompositeJdbcConsumer #857
Conversation
Turns out not all MinorTypes have a corresponding ArrowType, which can cause the following exception while handling the original exception: ``` Caused by: java.lang.UnsupportedOperationException: Cannot get simple type for type DECIMAL at org.apache.arrow.vector.types.Types$MinorType.getType(Types.java:815) at org.apache.arrow.adapter.jdbc.consumer.CompositeJdbcConsumer.consume(CompositeJdbcConsumer.java:49) ```
aiguofer
requested review from
lidavidm,
laurentgo,
wgtmac and
jbonofre
as code owners
aiguofer
changed the title
aiguofer
changed the title
|
|
||
| throw new JdbcConsumerException( | ||
| "Exception while consuming JDBC value", e, fieldInfo, arrowType); | ||
| "Exception while consuming JDBC value", e, fieldInfo, consumer.vector.getMinorType()); |
There was a problem hiding this comment.
Why not just vector.getField()? That'd be even better (you get the field name and type)
There was a problem hiding this comment.
I'm going to guess the original author relied a little too hard on IDE autocomplete and saw getMinorType but not getField
There was a problem hiding this comment.
haha I was the original author! 😂 As long as I've been working with Arrow in Java now, I can't say I fully understand all the different abstractions and their differences, so it's very likely getMinorType was the only thing I saw in auto-complete related to type.... But I definitely think Field would be better here! thanks for the suggestion
There was a problem hiding this comment.
ah, oops, sorry about that 😬
To be fair, I do think it's weird that we have a getMinorType but not a getType
What's Changed
Turns out not all MinorTypes have a corresponding ArrowType, which can cause the following exception while handling the original exception:
This PR changes the value we store in the exception to be the
MinorTypeinstead so we can still get useful info about the type but avoiding this possible exception.Closes #858.