Browse Source

SQL: improve ResultSet behavior when no rows are available (#46753)

Improve the defensive behavior of ResultSet when dealing with incorrect
API usage. In particular handle the case of dealing with no row
available (either because the cursor is before the first entry or after
the last).

Fix #46750
Costin Leau 6 years ago
parent
commit
58fa38e460

+ 8 - 3
x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java

@@ -37,8 +37,8 @@ import static org.elasticsearch.xpack.sql.jdbc.EsType.DATE;
 import static org.elasticsearch.xpack.sql.jdbc.EsType.DATETIME;
 import static org.elasticsearch.xpack.sql.jdbc.EsType.DATETIME;
 import static org.elasticsearch.xpack.sql.jdbc.EsType.TIME;
 import static org.elasticsearch.xpack.sql.jdbc.EsType.TIME;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.asDateTimeField;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.asDateTimeField;
-import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.dateTimeAsMillisSinceEpoch;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.asTimestamp;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.asTimestamp;
+import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.dateTimeAsMillisSinceEpoch;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.timeAsMillisSinceEpoch;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.timeAsMillisSinceEpoch;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.timeAsTime;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.timeAsTime;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.timeAsTimestamp;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.timeAsTimestamp;
@@ -57,6 +57,7 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
 
 
     private boolean closed = false;
     private boolean closed = false;
     private boolean wasNull = false;
     private boolean wasNull = false;
+    private boolean wasLast = false;
 
 
     private int rowNumber;
     private int rowNumber;
 
 
@@ -78,10 +79,13 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
         if (columnIndex < 1 || columnIndex > cursor.columnSize()) {
         if (columnIndex < 1 || columnIndex > cursor.columnSize()) {
             throw new SQLException("Invalid column index [" + columnIndex + "]");
             throw new SQLException("Invalid column index [" + columnIndex + "]");
         }
         }
+        if (wasLast == true || rowNumber < 1) {
+            throw new SQLException("No row available");
+        }
         Object object = null;
         Object object = null;
         try {
         try {
             object = cursor.column(columnIndex - 1);
             object = cursor.column(columnIndex - 1);
-        } catch (IllegalArgumentException iae) {
+        } catch (Exception iae) {
             throw new SQLException(iae.getMessage());
             throw new SQLException(iae.getMessage());
         }
         }
         wasNull = (object == null);
         wasNull = (object == null);
@@ -114,6 +118,7 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
             rowNumber++;
             rowNumber++;
             return true;
             return true;
         }
         }
+        wasLast = true;
         return false;
         return false;
     }
     }
 
 
@@ -461,7 +466,7 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
 
 
     @Override
     @Override
     public boolean isAfterLast() throws SQLException {
     public boolean isAfterLast() throws SQLException {
-        throw new SQLFeatureNotSupportedException("isAfterLast not supported");
+        return rowNumber > 0 && wasLast;
     }
     }
 
 
     @Override
     @Override

+ 28 - 0
x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java

@@ -1449,6 +1449,34 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
         assertThrowsWritesUnsupportedForUpdate(() -> r.rowDeleted());
         assertThrowsWritesUnsupportedForUpdate(() -> r.rowDeleted());
     }
     }
     
     
+    public void testResultSetNotInitialized() throws Exception {
+        createTestDataForNumericValueTypes(() -> randomInt());
+
+        SQLException sqle = expectThrows(SQLException.class, () -> {
+            doWithQuery(SELECT_WILDCARD, rs -> {
+                assertFalse(rs.isAfterLast());
+                rs.getObject(1);
+            });
+        });
+        assertEquals("No row available", sqle.getMessage());
+    }
+
+    public void testResultSetConsumed() throws Exception {
+        createTestDataForNumericValueTypes(() -> randomInt());
+
+        SQLException sqle = expectThrows(SQLException.class, () -> {
+            doWithQuery("SELECT * FROM test LIMIT 1", rs -> {
+                assertFalse(rs.isAfterLast());
+                assertTrue(rs.next());
+                assertFalse(rs.isAfterLast());
+                assertFalse(rs.next());
+                assertTrue(rs.isAfterLast());
+                rs.getObject(1);
+            });
+        });
+        assertEquals("No row available", sqle.getMessage());
+    }
+
     private void doWithQuery(String query, CheckedConsumer<ResultSet, SQLException> consumer) throws SQLException {
     private void doWithQuery(String query, CheckedConsumer<ResultSet, SQLException> consumer) throws SQLException {
         doWithQuery(() -> esJdbc(timeZoneId), query, consumer);
         doWithQuery(() -> esJdbc(timeZoneId), query, consumer);
     }
     }