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 <[email protected]> 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
