GH-858: Fix error handling in CompositeJdbcConsumer - #857
Conversation
Sorry, something went wrong.
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) ```
lidavidm
left a comment
There was a problem hiding this comment.
Technically this is a breaking change, not that I expect anyone to actually be constructing this exception themselves...
Sorry, something went wrong.
|
|
||
| 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)
Sorry, something went wrong.
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
Sorry, something went wrong.
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
Sorry, something went wrong.
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
Sorry, something went wrong.
lidavidm
left a comment
There was a problem hiding this comment.
Thanks!
Sorry, something went wrong.
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 MinorType instead so we can still get useful info about the type but avoiding this possible exception.
Closes #858.