lidavidm commented on code in PR #37085:
URL: https://github.com/apache/arrow/pull/37085#discussion_r1288968678


##########
java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowUtils.java:
##########
@@ -489,7 +489,7 @@ static JdbcConsumer getConsumer(ArrowType arrowType, int 
columnIndex, boolean nu
         return new NullConsumer((NullVector) vector);
       default:
         // no-op, shouldn't get here
-        throw new UnsupportedOperationException();
+        throw new UnsupportedOperationException("No consumer for Arrow type: " 
+ arrowType.getTypeID());

Review Comment:
   nit: might prefer just arrowType so we get the full toString?



##########
java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfig.java:
##########
@@ -338,4 +356,18 @@ public Map<Integer, Map<String, String>> 
getColumnMetadataByColumnIndex() {
   public RoundingMode getBigDecimalRoundingMode() {
     return bigDecimalRoundingMode;
   }
+
+  /**
+   * Interface for a function with 4 parameters.
+   *
+     * @param <A> param 1 type
+     * @param <B> param 2 type
+     * @param <C> param 3 type
+     * @param <D> param 4 type
+     * @param <R> return type
+   */
+  @FunctionalInterface
+  interface Function4Arity<A, B, C, D, R> {

Review Comment:
   1. public?
   2. Let's make this interface specific to the getter (so JdbcConsumerFactory 
or similar) so it's clear what it's for? (Also then it doesn't need to be 
generic.)



##########
java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/JdbcToArrowConfig.java:
##########
@@ -102,9 +105,10 @@ public final class JdbcToArrowConfig {
           Map<Integer, JdbcFieldInfo> arraySubTypesByColumnIndex,
           Map<String, JdbcFieldInfo> arraySubTypesByColumnName,
           int targetBatchSize,
-          Function<JdbcFieldInfo, ArrowType> jdbcToArrowTypeConverter) {
+          Function<JdbcFieldInfo, ArrowType> jdbcToArrowTypeConverter,
+          Function4Arity<ArrowType, Integer, Boolean, FieldVector, 
JdbcConsumer> jdbcConsumerGetter) {

Review Comment:
   Could we add a delegate constructor to avoid breaking existing code?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to