Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-718

Enable fetch to work for Statement.execute()

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4.0-incubating
    • Component/s: None
    • Labels:

      Description

      Current Avatica Statement.execute() will build the complete result set before sending back to client, enable fetch to send only max fetch rows.

      Reference discussion: CALCITE-712

        Issue Links

          Activity

          Hide
          jnadeau Jacques Nadeau added a comment -

          Resolved in release 1.4.0-incubating (2015-08-23)

          Show
          jnadeau Jacques Nadeau added a comment - Resolved in release 1.4.0-incubating (2015-08-23)
          Show
          julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/ec9d9660 . Thanks, Xavier!
          Hide
          xhoong Xavier FH Leong added a comment -

          Learn new things everyday, thanks Julian.

          Did a fixup and pushed.

          Show
          xhoong Xavier FH Leong added a comment - Learn new things everyday, thanks Julian. Did a fixup and pushed.
          Hide
          julianhyde Julian Hyde added a comment -

          Try emp cross join emp cross join emp.

          Show
          julianhyde Julian Hyde added a comment - Try emp cross join emp cross join emp.
          Hide
          xhoong Xavier FH Leong added a comment -

          Having a hard time finding a data source that is > 100 rows, as Scott data sets is too small, and FoodMart is too big. I ended up creating a derived table instead.

          Show
          xhoong Xavier FH Leong added a comment - Having a hard time finding a data source that is > 100 rows, as Scott data sets is too small, and FoodMart is too big. I ended up creating a derived table instead.
          Hide
          xhoong Xavier FH Leong added a comment -

          That's a good interface for testing JSON RPC, thanks.

          I'd picked your commit and added a test for testing PreparedStatement and Statement. Also throwing RuntimeException when no statement found from cache.

          Show
          xhoong Xavier FH Leong added a comment - That's a good interface for testing JSON RPC, thanks. I'd picked your commit and added a test for testing PreparedStatement and Statement. Also throwing RuntimeException when no statement found from cache.
          Hide
          julianhyde Julian Hyde added a comment -

          It's important that not only do we produce the right results, but we do it with the right requests and responses going over the RPC layer.

          I just added LoggingLocalJsonService and RequestLogger in https://github.com/julianhyde/incubator-calcite/tree/logging-json-service. If you run testPrepareBindExecuteFetch you should see it in action. Can you use that in your test and add some asserts that it is (for example) doing the right number of fetch requests.

          Show
          julianhyde Julian Hyde added a comment - It's important that not only do we produce the right results, but we do it with the right requests and responses going over the RPC layer. I just added LoggingLocalJsonService and RequestLogger in https://github.com/julianhyde/incubator-calcite/tree/logging-json-service . If you run testPrepareBindExecuteFetch you should see it in action. Can you use that in your test and add some asserts that it is (for example) doing the right number of fetch requests.
          Hide
          xhoong Xavier FH Leong added a comment -

          Replied to your comments.

          As for test case, when I was doing it, the existing test able to cover as it catches all the logic that I did not convert. As the fetch is internal logic, the front end logic test case tested the functionality once you convert from non-fetch to fetch.

          Show
          xhoong Xavier FH Leong added a comment - Replied to your comments. As for test case, when I was doing it, the existing test able to cover as it catches all the logic that I did not convert. As the fetch is internal logic, the front end logic test case tested the functionality once you convert from non-fetch to fetch.
          Hide
          julianhyde Julian Hyde added a comment -

          See my comments on your pull request.

          Also, I don't see any new tests. Which tests prove that you've fixed the bug?

          Show
          julianhyde Julian Hyde added a comment - See my comments on your pull request. Also, I don't see any new tests. Which tests prove that you've fixed the bug?
          Hide
          xhoong Xavier FH Leong added a comment -
          Show
          xhoong Xavier FH Leong added a comment - Pull request created: https://github.com/apache/incubator-calcite/pull/88
          Show
          xhoong Xavier FH Leong added a comment - WIP branch: https://github.com/xhoong/incubator-calcite/tree/718-cursorFetch I'm combining with CALCITE-712
          Hide
          julianhyde Julian Hyde added a comment -

          Agreed. Can you please log an issue for this.

          Show
          julianhyde Julian Hyde added a comment - Agreed. Can you please log an issue for this.
          Hide
          xhoong Xavier FH Leong added a comment -

          I have started looking into this, the main reason is that when the frame is created in JdbcResultSet, it is hardcoded to all rows (-1), that can change to in sync with FetchIterable value which is 100.

          However, there's also a logic flow problem, in Avatica, every createStatement call is also RPC to the server, but the remote statement originally created was never used, and each call to Statement.execute() will actually create a brand new statement, hence the statement ID on remote vs local is out of sync during fetch. I plan to modify the parameter for prepareAndExecute to use StatementHandle instead of ConnectionHandle (where it store both the connection and statement ID) to recover the previous created statement.

          Currently, the statement and connection cache are remain separate, I see a problem during invalidation of cache if they are kept apart, connection may close first before statement and making statement retrieved from statement cache being staled. One way is to extend the connection cache to include data structure that also store statement map, and there only remain a single cache policy. This is most likely a separate issue from this JIRA.

          Show
          xhoong Xavier FH Leong added a comment - I have started looking into this, the main reason is that when the frame is created in JdbcResultSet, it is hardcoded to all rows (-1), that can change to in sync with FetchIterable value which is 100. However, there's also a logic flow problem, in Avatica, every createStatement call is also RPC to the server, but the remote statement originally created was never used, and each call to Statement.execute() will actually create a brand new statement, hence the statement ID on remote vs local is out of sync during fetch. I plan to modify the parameter for prepareAndExecute to use StatementHandle instead of ConnectionHandle (where it store both the connection and statement ID) to recover the previous created statement. Currently, the statement and connection cache are remain separate, I see a problem during invalidation of cache if they are kept apart, connection may close first before statement and making statement retrieved from statement cache being staled. One way is to extend the connection cache to include data structure that also store statement map, and there only remain a single cache policy. This is most likely a separate issue from this JIRA.

            People

            • Assignee:
              julianhyde Julian Hyde
              Reporter:
              xhoong Xavier FH Leong
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development