[GitHub] [arrow] toddfarmer commented on a diff in pull request #13166: ARROW-16427: [Java] Provide explicit column type mapping

[GitBox <gi...@apache.org>]

toddfarmer commented on code in PR #13166:
URL: https://github.com/apache/arrow/pull/13166#discussion_r875139658


##########
java/adapter/jdbc/src/test/java/org/apache/arrow/adapter/jdbc/h2/JdbcToArrowTest.java:
##########
@@ -311,4 +320,146 @@ public void testMockDataTypes() throws SQLException {
     assertEquals("1", element.getString());
   }
 
+  @Test
+  public void testUnreliableMetaDataPrecisionAndScale() throws Exception {
+    BufferAllocator allocator = new RootAllocator(Integer.MAX_VALUE);
+    int x = 0;
+    final int targetRows = 0;
+    ResultSet rs = buildIncorrectPrecisionAndScaleMetaDataResultSet();
+    ResultSetMetaData rsmd = rs.getMetaData();
+    assertEquals("Column type should be Types.DECIMAL", Types.DECIMAL, rsmd.getColumnType(1));
+    assertEquals("Column scale should be zero", 0, rsmd.getScale(1));
+    assertEquals("Column precision should be zero", 0, rsmd.getPrecision(1));
+    rs.next();
+    BigDecimal bd1 = rs.getBigDecimal(1);
+    assertEquals("Value should be 1000000000000000.01", new BigDecimal("1000000000000000.01"), bd1);
+    assertEquals("Value scale should be 2", 2, bd1.scale());
+    assertEquals("Value precision should be 18", 18, bd1.precision());
+    assertFalse("No more rows!", rs.next());
+
+    // reset the ResultSet:
+    rs.beforeFirst();
+    JdbcToArrowConfig config = new JdbcToArrowConfigBuilder(
+          allocator, JdbcToArrowUtils.getUtcCalendar(), /* include metadata */ false)
+          .setReuseVectorSchemaRoot(reuseVectorSchemaRoot)
+          .build();
+    try {
+      ArrowVectorIterator iter = JdbcToArrow.sqlToArrowVectorIterator(rs, config);
+      while (iter.hasNext()) {
+        iter.next();
+      }
+      fail("Expected to fail due to mismatched metadata!");
+      iter.close();
+    } catch (Exception ex) {
+      // expected to fail
+    }
+
+    // reset the ResultSet:
+    rs.beforeFirst();
+    JdbcFieldInfo explicitMappingField = new JdbcFieldInfo(Types.DECIMAL, 18, 2);
+    Map<Integer, JdbcFieldInfo> explicitMapping = new HashMap<>();
+    explicitMapping.put(1, explicitMappingField);
+    config = new JdbcToArrowConfigBuilder(
+            allocator, JdbcToArrowUtils.getUtcCalendar(), /* include metadata */ false)
+            .setReuseVectorSchemaRoot(reuseVectorSchemaRoot)
+            .setExplicitTypesByColumnIndex(explicitMapping)
+            .build();
+
+    try {
+      ArrowVectorIterator iter = JdbcToArrow.sqlToArrowVectorIterator(rs, config);
+      while (iter.hasNext()) {
+        iter.next();
+      }
+      iter.close();
+    } catch (Exception ex) {
+      fail("Should not fail with explicit metadata supplied!");
+    }
+
+  }
+
+  @Test
+  public void testInconsistentPrecisionAndScale() throws Exception {
+    BufferAllocator allocator = new RootAllocator(Integer.MAX_VALUE);
+    int x = 0;
+    final int targetRows = 0;
+    ResultSet rs = buildVaryingPrecisionAndScaleResultSet();
+    ResultSetMetaData rsmd = rs.getMetaData();
+    assertEquals("Column type should be Types.DECIMAL", Types.DECIMAL, rsmd.getColumnType(1));
+    assertEquals("Column scale should be zero", 0, rsmd.getScale(1));
+    assertEquals("Column precision should be zero", 0, rsmd.getPrecision(1));
+    rs.next();
+    BigDecimal bd1 = rs.getBigDecimal(1);
+    assertEquals("Value should be 1000000000000000.01", new BigDecimal("1000000000000000.01"), bd1);
+    assertEquals("Value scale should be 2", 2, bd1.scale());
+    assertEquals("Value precision should be 18", 18, bd1.precision());
+    rs.next();
+    BigDecimal bd2 = rs.getBigDecimal(1);
+    assertEquals("Value should be 1000000000300.0000001", new BigDecimal("1000000000300.0000001"), bd2);
+    assertEquals("Value scale should be 7", 7, bd2.scale());
+    assertEquals("Value precision should be 20", 20, bd2.precision());
+    rs.beforeFirst();
+    JdbcFieldInfo explicitMappingField = new JdbcFieldInfo(Types.DECIMAL, 20, 7);
+    Map<Integer, JdbcFieldInfo> explicitMapping = new HashMap<>();
+    explicitMapping.put(1, explicitMappingField);
+
+    JdbcToArrowConfig config = new JdbcToArrowConfigBuilder(
+            allocator, JdbcToArrowUtils.getUtcCalendar(), /* include metadata */ false)
+            .setReuseVectorSchemaRoot(reuseVectorSchemaRoot)
+            .setExplicitTypesByColumnIndex(explicitMapping)
+            .build();
+    try {
+      ArrowVectorIterator iter = JdbcToArrow.sqlToArrowVectorIterator(rs, config);
+      while (iter.hasNext()) {
+        iter.next();
+      }
+      iter.close();
+    } catch (Exception ex) {
+      // Fails here due to ARROW-16427:
+      // fail("Failed to process ResultSet");

Review Comment:
   It does fail today.  My proposal is to break out the task to enable configurable automatic coercion to greater-scale BigDecimals in a separate task.  If you agree, I'll open a new task, update the comment to reference that issue, and start work on that after this is closed.  This work only allows explicit type mapping via configuration for known-consistent ResultSets.  Further work is needed to enable support for inconsistent ResultSets, where scale differs by row.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org