Skip to content

GH-858: Fix error handling in CompositeJdbcConsumer #857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lidavidm merged 2 commits into from
Sep 25, 2025
Merged

GH-858: Fix error handling in CompositeJdbcConsumer #857

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9bb4214
Fix error handling in CompositeJdbcConsumer
aiguofer Sep 24, 2025
d7675b6
Change MinorType -> Field
aiguofer Sep 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/consumer/CompositeJdbcConsumer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.apache.arrow.util.AutoCloseables;
import org.apache.arrow.vector.ValueVector;
import org.apache.arrow.vector.VectorSchemaRoot;
import org.apache.arrow.vector.types.pojo.ArrowType;

/** Composite consumer which hold all consumers. It manages the consume and cleanup process. */
public class CompositeJdbcConsumer implements JdbcConsumer {
Expand All @@ -46,9 +45,9 @@ public void consume(ResultSet rs) throws SQLException, IOException {
BaseConsumer consumer = (BaseConsumer) consumers[i];
JdbcFieldInfo fieldInfo =
new JdbcFieldInfo(rs.getMetaData(), consumer.columnIndexInResultSet);
ArrowType arrowType = consumer.vector.getMinorType().getType();

throw new JdbcConsumerException(
"Exception while consuming JDBC value", e, fieldInfo, arrowType);
"Exception while consuming JDBC value", e, fieldInfo, consumer.vector.getMinorType());
Copy link
Member

@lidavidm lidavidm Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just vector.getField()? That'd be even better (you get the field name and type)

Copy link
Member

@lidavidm lidavidm Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to guess the original author relied a little too hard on IDE autocomplete and saw getMinorType but not getField

Copy link
Contributor Author

@aiguofer aiguofer Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@lidavidm lidavidm Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, oops, sorry about that 😬

To be fair, I do think it's weird that we have a getMinorType but not a getType

} else {
throw e;
}
Expand Down
14 changes: 7 additions & 7 deletions ...rc/main/java/org/apache/arrow/adapter/jdbc/consumer/exceptions/JdbcConsumerException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,33 +17,33 @@
package org.apache.arrow.adapter.jdbc.consumer.exceptions;

import org.apache.arrow.adapter.jdbc.JdbcFieldInfo;
import org.apache.arrow.vector.types.pojo.ArrowType;
import org.apache.arrow.vector.types.Types;

/**
* Exception while consuming JDBC data. This exception stores the JdbcFieldInfo for the column and
* the ArrowType for the corresponding vector for easier debugging.
*/
public class JdbcConsumerException extends RuntimeException {
final JdbcFieldInfo fieldInfo;
final ArrowType arrowType;
final Types.MinorType minorType;

/**
* Construct JdbcConsumerException with all fields.
*
* @param message error message
* @param cause original exception
* @param fieldInfo JdbcFieldInfo for the column
* @param arrowType ArrowType for the corresponding vector
* @param minorType ArrowType for the corresponding vector
*/
public JdbcConsumerException(
String message, Throwable cause, JdbcFieldInfo fieldInfo, ArrowType arrowType) {
String message, Throwable cause, JdbcFieldInfo fieldInfo, Types.MinorType minorType) {
super(message, cause);
this.fieldInfo = fieldInfo;
this.arrowType = arrowType;
this.minorType = minorType;
}

public ArrowType getArrowType() {
return this.arrowType;
public Types.MinorType getMinorType() {
return this.minorType;
}

public JdbcFieldInfo getFieldInfo() {
Expand Down