GH-39355: [Java] Improve JdbcConsumer exceptions

[aiguofer]

Rationale for this change

This helps debug Arrow conversion errors from JDBC by exposing the JdbcFieldInfo for the related column and the ArrowType for the corresponding vector.

What changes are included in this PR?

A new JdbcConsumerException which is thrown by the CompositeJdbcConsumer while consuming data for a specific vector.

Are these changes tested?

N/A

Are there any user-facing changes?

Users can now catch JdbcConsumerExceptions and extract the related ArrowType and JdbcFieldInfo from it for debugging.

• Closes: [Java] Improve JdbcConsumer errors #39355

[github-actions]

⚠️ GitHub issue #39355 has been automatically assigned in GitHub to PR creator.

[davisusanibar]

Thanks to @aiguofer for this enhancement. Would it be possible to add a unit test to cover these new exception improvements?

[vibhatha]

@aiguofer agreeing with @davisusanibar here.
It would be better to add a test case which is failing as expected and verify the exception message as an assertion.
Also just curious would this require documentation update?

[aiguofer]

@davisusanibar @vibhatha It would certainly be great to have tests for consumer! unfortunately there's not much prior art regarding this. I'm not very familiar with the test suite in general and this would likely require mocking a ResultSet along with the corresponding ResultSetMetaData in order to properly test this, which seems like a lot of work.

[lidavidm]

For testing, we do use hsqldb in the suite to integration test against an actual database (instead of mocking everything). That said mocking may be easier here overall

[lidavidm]

Also just curious would this require documentation update?

It would be good to add a @throws in the docstring for ArrowVectorIterator. Actually, this might be considered a breaking change since people may expect SQLException currently. (And it's documented in sqlToArrowVectorIterator.)

Possibly we should extend SQLException instead?

[vibhatha]

Possibly we should extend SQLException instead?

+1

[vibhatha]

For testing, we do use hsqldb in the suite to integration test against an actual database (instead of mocking everything). That said mocking may be easier here overall

should we include it in this PR itself or should we create follow up?

[aiguofer]

Sorry all, I'm just getting back from break. The exceptions are not always a SQLException, they can also be a conversion error. For example, in our case what prompted this PR was this redacted stack trace:

java.lang.RuntimeException: Error occurred while getting next schema root.
	at org.apache.arrow.adapter.jdbc.ArrowVectorIterator.next(ArrowVectorIterator.java:181)
        ...
Caused by: java.lang.RuntimeException: Error occurred while consuming data.
	at org.apache.arrow.adapter.jdbc.ArrowVectorIterator.consumeData(ArrowVectorIterator.java:117)
	at org.apache.arrow.adapter.jdbc.ArrowVectorIterator.load(ArrowVectorIterator.java:159)
	at org.apache.arrow.adapter.jdbc.ArrowVectorIterator.next(ArrowVectorIterator.java:177)
	... 10 common frames omitted
Caused by: java.lang.UnsupportedOperationException: Decimal size greater than 16 bytes: 17
	at org.apache.arrow.vector.util.DecimalUtility.writeByteArrayToArrowBufHelper(DecimalUtility.java:175)
	at org.apache.arrow.vector.util.DecimalUtility.writeBigDecimalToArrowBuf(DecimalUtility.java:136)
	at org.apache.arrow.vector.DecimalVector.set(DecimalVector.java:365)
	at org.apache.arrow.adapter.jdbc.consumer.DecimalConsumer.set(DecimalConsumer.java:79)
	at org.apache.arrow.adapter.jdbc.consumer.DecimalConsumer$NullableDecimalConsumer.consume(DecimalConsumer.java:101)
	at org.apache.arrow.adapter.jdbc.consumer.CompositeJdbcConsumer.consume(CompositeJdbcConsumer.java:46)
	at org.apache.arrow.adapter.jdbc.ArrowVectorIterator.consumeData(ArrowVectorIterator.java:106)
	... 12 common frames omitted

Currently, the JdbcConsumerException extends a RuntimeException so no throws is necessary is non-breaking. It could potentially be useful to extend Exception instead and add throws, but to your point that'd be a breaking change.

[lidavidm]

Ok, thanks. Then I think just documenting it in @throws is sufficient.

[aiguofer]

@lidavidm done!

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit a71c941.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 5 possible false positives for unstable benchmarks that are known to sometimes produce them.