[CALCITE-2955] SQL queries after first query in same statement have no results returned

[leesf]

see jira:CALCITE-2955

[hsyuan]

Is it possible to add a test case for this?

[hsyuan]

Also check CI failure

[leesf]

@hsyuan hi, the travis shows the error in RexProgramFuzzyTest#checkUnknownAs, but it seems this PR does not change the file. The Travis log link is Travis-ci-log, any suggestions?

[hsyuan]

reopen the pr to trigger CI

[zhztheplayer]

This is too aggressive :(

The iterator is not released after a call ended because it might not be fully read. If I'm not wrong, setting it to null would cause infinite loop when result row count is large enough.

[leesf]

Thanks for your review and sorry for lately reply @zhztheplayer , updated the PR to address your comment.

[zhztheplayer]

Thanks for the changes @leesf. And could you please add a unit test to guard against large result set? Now that the fetch size in Avatica is defaulted to 100[1]. You can make the rows of return larger than the number.

[1] https://github.com/apache/calcite-avatica/blob/96507bfe737f2188c16dec9d16d5e8b502df231f/core/src/main/java/org/apache/calcite/avatica/AvaticaStatement.java#L40

[zhztheplayer]

The logic looks still vulnerable. If the first iterator is not "done", then we execute second query the result will be wrong. I think the correct fix should relate to the CLOSE phase of ResultSet instances in client side, but I tested against calling resultSet.close() before executing the second query and nothing happened. Maybe something was wrong there.

[leesf]

@zhztheplayer Thanks for you quick look again. And i don't know what you means

calling resultSet.close() before executing the second query and nothing happened

Since I think there is no need to call ResultSet#close.

And I add an unit test for fetching more than 100 result. Please take a look. Thanks.

[zhztheplayer]

It seems that stmt.setResultSet(null) should be called by the operation of closing the ResultSet object on client side (we can treat the re-executing of Statement object implicitly closes the current ResultSet opened by it), because inside the method CalciteMetaImpl#fetch we can barely know exactly when we should execute a new query (please correct me if I am wrong).

For example, you can check following test case:

 @Test public void testFoo() throws Exception {
   try (Connection remoteConnection = getRemoteConnection()) {
     final Statement statement = remoteConnection.createStatement();
     StringBuffer stringBuffer = new StringBuffer();
     stringBuffer.append("values ");
     final int resultSize = 1000;
     for (int i = 0; i < resultSize; i++) {
       stringBuffer.append("(").append(i).append(", ").append(i + 1).append(")");
       if (i != resultSize - 1) {
         stringBuffer.append(", ");
       }
     }
     final String sql = stringBuffer.toString();
     ResultSet resultSet = statement.executeQuery(sql);

     resultSet = statement.executeQuery("values (1, 'a')");
     int n = 0;
     while (resultSet.next()) {
       ++n;
     }
     assertThat(n, equalTo(1));
   }
 }