Github user kkhatua commented on a diff in the pull request:
https://github.com/apache/drill/pull/1024#discussion_r150157923
--- 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 --
I don't think you are wrong, but I think the interpretation of the timeout
is ambiguous. My understanding based on what drivers like Oracle do is to start
the timeout only when the execute call is made. So, for a regular Statement
object, just initialization (or even setting the timeout) should not be the
basis of starting the timer.
With regards to whether we are testing for the time when only the
DrillCursor is in operation, we'd need a query that is running sufficiently
long to timeout before the server can send back anything for the very first
time. The `awaitFirstMessage()` already has the timeout applied there and
worked in some of my longer running sample queries. If you're hinting towards
this, then yes.. it is certainly doesn't hurt to have the test, although the
timeout does guarantee exactly that.
I'm not familiar with the Drillbit Injection feature, so let me tinker a
bit to confirm it before I update the PR.
---
