You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@drill.apache.org by ih...@apache.org on 2019/08/05 07:38:49 UTC

[drill] 01/04: DRILL-7084: ResultSet getObject method throws not implemented exception if the column type is NULL

This is an automated email from the ASF dual-hosted git repository.

ihuzenko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/drill.git

commit 22947ad336d8dd0d944a66ded16a7ce9ffa62f04
Author: Anton Gozhiy <an...@gmail.com>
AuthorDate: Tue Jul 9 18:04:52 2019 +0300

    DRILL-7084: ResultSet getObject method throws not implemented exception if the column type is NULL
    
    closes #1825
---
 .../apache/drill/jdbc/impl/DrillResultSetImpl.java |  24 ++---
 .../org/apache/drill/jdbc/DrillResultSetTest.java  | 109 ++++++++++-----------
 .../src/test/resources/testGetObjectNull.parquet   | Bin 0 -> 379 bytes
 3 files changed, 65 insertions(+), 68 deletions(-)

diff --git a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
index 5948393..0ce1e79 100644
--- a/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
+++ b/exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java
@@ -78,11 +78,11 @@ public class DrillResultSetImpl extends AvaticaResultSet implements DrillResultS
    * Throws AlreadyClosedSqlException or QueryCanceledSqlException if this
    * ResultSet is closed.
    *
-   * @throws  ExecutionCanceledSqlException  if ResultSet is closed because of
-   *          cancelation and no QueryCanceledSqlException has been thrown yet
-   *          for this ResultSet
-   * @throws  AlreadyClosedSqlException  if ResultSet is closed
-   * @throws  SQLException  if error in calling {@link #isClosed()}
+   * @throws ExecutionCanceledSqlException if ResultSet is closed because of
+   *         cancellation and no QueryCanceledSqlException has been thrown yet
+   *         for this ResultSet
+   * @throws AlreadyClosedSqlException if ResultSet is closed
+   * @throws SQLException if error in calling {@link #isClosed()}
    */
   @Override
   protected void checkOpen() throws SQLException {
@@ -168,7 +168,7 @@ public class DrillResultSetImpl extends AvaticaResultSet implements DrillResultS
   }
 
   @Override
-  public Object getObject( int columnIndex ) throws SQLException {
+  public Object getObject(int columnIndex) throws SQLException {
     checkOpen();
 
     Cursor.Accessor accessor;
@@ -180,7 +180,7 @@ public class DrillResultSetImpl extends AvaticaResultSet implements DrillResultS
     ColumnMetaData metaData = columnMetaDataList.get(columnIndex - 1);
     // Drill returns a float (4bytes) for a SQL Float whereas Calcite would return a double (8bytes)
     int typeId = (metaData.type.id != Types.FLOAT) ? metaData.type.id : Types.REAL;
-    return AvaticaSite.get(accessor, typeId, localCalendar);
+    return typeId != Types.NULL ? AvaticaSite.get(accessor, typeId, localCalendar) : null;
   }
 
   //--------------------------JDBC 2.0-----------------------------------
@@ -249,10 +249,10 @@ public class DrillResultSetImpl extends AvaticaResultSet implements DrillResultS
   }
 
   @Override
-  public boolean relative( int rows ) throws SQLException {
+  public boolean relative(int rows) throws SQLException {
     checkOpen();
     try {
-      return super.relative( rows );
+      return super.relative(rows);
     } catch (UnsupportedOperationException e) {
       throw new SQLFeatureNotSupportedException(e.getMessage(), e);
     }
@@ -275,7 +275,7 @@ public class DrillResultSetImpl extends AvaticaResultSet implements DrillResultS
   public void updateNull(int columnIndex) throws SQLException {
     checkOpen();
     try {
-      super.updateNull( columnIndex );
+      super.updateNull(columnIndex);
     } catch (UnsupportedOperationException e) {
       throw new SQLFeatureNotSupportedException(e.getMessage(), e);
     }
@@ -295,7 +295,7 @@ public class DrillResultSetImpl extends AvaticaResultSet implements DrillResultS
   public void updateByte(int columnIndex, byte x) throws SQLException {
     checkOpen();
     try {
-      super.updateByte( columnIndex, x );
+      super.updateByte(columnIndex, x);
     } catch (UnsupportedOperationException e) {
       throw new SQLFeatureNotSupportedException(e.getMessage(), e);
     }
@@ -1260,7 +1260,7 @@ public class DrillResultSetImpl extends AvaticaResultSet implements DrillResultS
   ////////////////////////////////////////
 
   @Override
-  protected DrillResultSetImpl execute() throws SQLException{
+  protected DrillResultSetImpl execute() throws SQLException {
     connection.getDriver().handler.onStatementExecute(statement, null);
 
     if (signature.cursorFactory != null) {
diff --git a/exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java b/exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java
index e75d19c..73f96e5 100644
--- a/exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java
+++ b/exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java
@@ -18,11 +18,10 @@
 package org.apache.drill.jdbc;
 
 import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.core.StringContains.containsString;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
 
 import java.sql.Connection;
 import java.sql.ResultSet;
@@ -34,8 +33,10 @@ import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.categories.JdbcTest;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.junit.rules.ExpectedException;
 
 @Category({SlowTest.class, JdbcTest.class})
 public class DrillResultSetTest extends JdbcTestBase {
@@ -45,123 +46,119 @@ public class DrillResultSetTest extends JdbcTestBase {
       ExecConstants.HTTP_ENABLE;
 
   private static final String origStatusServerPropValue =
-      System.getProperty( STATUS_SERVER_PROPERTY_NAME, "true" );
+      System.getProperty(STATUS_SERVER_PROPERTY_NAME, "true");
+
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
 
   // Disable Jetty status server so unit tests run (outside Maven setup).
   // (TODO:  Move this to base test class and/or have Jetty try other ports.)
   @BeforeClass
   public static void setUpClass() {
-    System.setProperty( STATUS_SERVER_PROPERTY_NAME, "false" );
+    System.setProperty(STATUS_SERVER_PROPERTY_NAME, "false");
   }
 
   @AfterClass
   public static void tearDownClass() {
-    System.setProperty( STATUS_SERVER_PROPERTY_NAME, origStatusServerPropValue );
+    System.setProperty(STATUS_SERVER_PROPERTY_NAME, origStatusServerPropValue);
   }
 
 
   @Test
   public void test_next_blocksFurtherAccessAfterEnd()
-      throws SQLException
-  {
+      throws SQLException {
     Connection connection = connect();
     Statement statement = connection.createStatement();
     ResultSet resultSet =
-        statement.executeQuery( "SELECT 1 AS x \n" +
-                                "FROM cp.`donuts.json` \n" +
-                                "LIMIT 2" );
+        statement.executeQuery("SELECT 1 AS x \n" +
+            "FROM cp.`donuts.json` \n" +
+            "LIMIT 2");
 
     // Advance to first row; confirm can access data.
-    assertThat( resultSet.next(), is( true ) );
-    assertThat( resultSet.getInt( 1 ), is ( 1 ) );
+    assertThat(resultSet.next(), is(true));
+    assertThat(resultSet.getInt(1), is(1));
 
     // Advance from first to second (last) row, confirming data access.
-    assertThat( resultSet.next(), is( true ) );
-    assertThat( resultSet.getInt( 1 ), is ( 1 ) );
+    assertThat(resultSet.next(), is(true));
+    assertThat(resultSet.getInt(1), is(1));
 
     // Now advance past last row.
-    assertThat( resultSet.next(), is( false ) );
+    assertThat(resultSet.next(), is(false));
 
     // Main check:  That row data access methods now throw SQLException.
-    try {
-      resultSet.getInt( 1 );
-      fail( "Didn't get expected SQLException." );
-    }
-    catch ( SQLException e ) {
-      // Expect something like current InvalidCursorStateSqlException saying
-      // "Result set cursor is already positioned past all rows."
-      assertThat( e, instanceOf( InvalidCursorStateSqlException.class ) );
-      assertThat( e.toString(), containsString( "past" ) );
-    }
-    // (Any other exception is unexpected result.)
-
-    assertThat( resultSet.next(), is( false ) );
+    thrown.expect(InvalidCursorStateSqlException.class);
+    thrown.expectMessage(containsString("Result set cursor is already positioned past all rows."));
+    resultSet.getInt(1);
+
+    assertThat(resultSet.next(), is(false));
 
     // TODO:  Ideally, test all other accessor methods.
   }
 
   @Test
   public void test_next_blocksFurtherAccessWhenNoRows()
-    throws Exception
-  {
+      throws Exception {
     Connection connection = connect();
     Statement statement = connection.createStatement();
     ResultSet resultSet =
-        statement.executeQuery( "SELECT 'Hi' AS x \n" +
-                                "FROM cp.`donuts.json` \n" +
-                                "WHERE false" );
+        statement.executeQuery("SELECT 'Hi' AS x \n" +
+            "FROM cp.`donuts.json` \n" +
+            "WHERE false");
 
     // Do initial next(). (Advance from before results to next possible
     // position (after the set of zero rows).
-    assertThat( resultSet.next(), is( false ) );
+    assertThat(resultSet.next(), is(false));
 
     // Main check:  That row data access methods throw SQLException.
-    try {
-      resultSet.getString( 1 );
-      fail( "Didn't get expected SQLException." );
-    }
-    catch ( SQLException e ) {
-      // Expect something like current InvalidRowSQLException saying
-      // "Result set cursor is already positioned past all rows."
-      assertThat( e, instanceOf( InvalidCursorStateSqlException.class ) );
-      assertThat( e.toString(), containsString( "past" ) );
-      assertThat( e.toString(), containsString( "rows" ) );
-    }
-    // (Any non-SQLException exception is unexpected result.)
-
-    assertThat( resultSet.next(), is( false ) );
+    thrown.expect(InvalidCursorStateSqlException.class);
+    thrown.expectMessage(containsString("Result set cursor is already positioned past all rows."));
+    resultSet.getString(1);
+
+    assertThat(resultSet.next(), is(false));
 
     // TODO:  Ideally, test all other accessor methods.
   }
 
   @Test
   public void test_getRow_isOneBased()
-    throws Exception
-  {
+      throws Exception {
     Connection connection = connect();
     Statement statement = connection.createStatement();
     ResultSet resultSet =
-        statement.executeQuery( "VALUES (1), (2)" );
+        statement.executeQuery("VALUES (1), (2)");
 
     // Expect 0 when before first row:
-    assertThat( "getRow() before first next()", resultSet.getRow(), equalTo( 0 ) );
+    assertThat("getRow() before first next()", resultSet.getRow(), equalTo(0));
 
     resultSet.next();
 
     // Expect 1 at first row:
-    assertThat( "getRow() at first row", resultSet.getRow(), equalTo( 1 ) );
+    assertThat("getRow() at first row", resultSet.getRow(), equalTo(1));
 
     resultSet.next();
 
     // Expect 2 at second row:
-    assertThat( "getRow() at second row", resultSet.getRow(), equalTo( 2 ) );
+    assertThat("getRow() at second row", resultSet.getRow(), equalTo(2));
 
     resultSet.next();
 
     // Expect 0 again when after last row:
-    assertThat( "getRow() after last row", resultSet.getRow(), equalTo( 0 ) );
+    assertThat("getRow() after last row", resultSet.getRow(), equalTo(0));
+    resultSet.next();
+    assertThat("getRow() after last row", resultSet.getRow(), equalTo(0));
+  }
+
+  @Test
+  public void testGetObjectNull() throws SQLException {
+    Connection connection = connect();
+    Statement statement = connection.createStatement();
+    ResultSet resultSet =
+        statement.executeQuery(
+            "select coalesce(a1, b1) " +
+            "from cp.`testGetObjectNull.parquet` " +
+            "limit 1");
     resultSet.next();
-    assertThat( "getRow() after last row", resultSet.getRow(), equalTo( 0 ) );
+    assertNull(resultSet.getObject(1));
   }
 
   // TODO:  Ideally, test other methods.
diff --git a/exec/jdbc/src/test/resources/testGetObjectNull.parquet b/exec/jdbc/src/test/resources/testGetObjectNull.parquet
new file mode 100644
index 0000000..f3cfefd
Binary files /dev/null and b/exec/jdbc/src/test/resources/testGetObjectNull.parquet differ