Github user laurentgo commented on a diff in the pull request:
https://github.com/apache/drill/pull/1024#discussion_r149543581
--- Diff:
exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java ---
@@ -96,6 +105,14 @@ private void throwIfClosed() throws
AlreadyClosedSqlException,
throw new AlreadyClosedSqlException( "ResultSet is already
closed." );
}
}
+
+ //Implicit check for whether timeout is set
+ if (elapsedTimer != null) {
--- End diff --
maybe an helper method from the cursor to see if we timed out instead of
exposing elapsedTimer?
I'm not sure if this is really necessary (posted another comment about it
previously), except maybe because of unit tests where it's hard to time out
inside the cursor?
I did a prototype too and used control injection to pause the screen
operator: the test would look like this:
```
/**
* Test setting timeout for a query that actually times out
*/
@Test ( expected = SqlTimeoutException.class )
public void testTriggeredQueryTimeout() throws SQLException {
// Prevent the server to complete the query to trigger a timeout
final String controls = Controls.newBuilder()
.addPause(ScreenCreator.class, "send-complete", 0)
.build();
try(Statement statement = connection.createStatement()) {
assertThat(
statement.execute(String.format(
"ALTER session SET `%s` = '%s'",
ExecConstants.DRILLBIT_CONTROL_INJECTIONS,
controls)),
equalTo(true));
}
String queryId = null;
try(Statement statement = connection.createStatement()) {
int timeoutDuration = 3;
//Setting to a very low value (3sec)
statement.setQueryTimeout(timeoutDuration);
ResultSet rs = statement.executeQuery(SYS_VERSION_SQL);
queryId = ((DrillResultSet) rs).getQueryId();
//Fetch rows
while (rs.next()) {
rs.getBytes(1);
}
} catch (SQLException sqlEx) {
if (sqlEx instanceof SqlTimeoutException) {
throw (SqlTimeoutException) sqlEx;
}
} finally {
// Do not forget to unpause to avoid memory leak.
if (queryId != null) {
DrillClient drillClient = ((DrillConnection)
connection).getClient();
drillClient.resumeQuery(QueryIdHelper.getQueryIdFromString(queryId));
}
}
```
Works for PreparedStatementTest too, need to make sure you pause after
prepared statement is created but before it is executed.
---
