kdelay opened a new pull request, #19941: URL: https://github.com/apache/druid/pull/19941
Fixes #19918. ### Description A JDBC client that calls `Statement.setMaxRows(n)` still gets every row the query produces. Avatica sends the value to `DruidMeta.prepareAndExecute` as `maxRowCount`, but `DruidJdbcStatement.execute()` discarded it and constructed the result set with `Long.MAX_VALUE`. #### Passed the received row limit through `DruidJdbcStatement.execute()` now hands `maxRowCount` to `DruidJdbcResultSet` instead of `Long.MAX_VALUE`. `DruidJdbcPreparedStatement` already did this, so this makes the two statement types consistent. Unlimited statements are unaffected. `AvaticaStatement.executeInternal` converts the JDBC "no limit" value to `-1` before the request leaves the client (`maxRowCount <= 0 ? -1 : maxRowCount`), and `DruidJdbcResultSet.execute()` already reads a negative limit as `Integer.MAX_VALUE`. So the only values that reach the fetcher as a real cap are the ones the user asked for. #### Marked a frame complete when the row limit is reached `DruidJdbcResultSet.ResultFetcher.call()` set `Meta.Frame.done` from `yielder.isDone()` alone. When the limit is lower than the number of rows the query produces, the yielder is not exhausted at the limit, so the last in-limit frame reported `done = false`. The next fetch then computes `batchLimit = min(limit - offset, batchSize) = 0`, returns an empty frame that is again not done, and the client keeps fetching forever. The frame is now also complete once `offset + rowCount` reaches the limit. #### Not covered: prepared statements The reporter's example uses a `PreparedStatement`, and that path still does not honor the limit. Avatica calls `Meta.prepare(..., -1)` when the statement is created and passes only the fetch size at execution time, so `PreparedStatement.getMaxRows()` never reaches the server. That is [CALCITE-719](https://issues.apache.org/jira/browse/CALCITE-719), still open upstream. Adding an explicit `LIMIT` to the SQL remains the workaround there. This PR fixes what Druid can fix on its own. #### Release note The Avatica JDBC server now honors `Statement.setMaxRows()`. Previously the setting was ignored and the full result was returned. `PreparedStatement.setMaxRows()` is still not honored because the value is not transmitted to the server (CALCITE-719). <hr> ##### Key changed/added classes in this PR * `DruidJdbcStatement` * `DruidJdbcResultSet` <hr> This PR has: - [x] been self-reviewed. - [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader. - [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met. Two tests were added to `DruidStatementTest`: `testMaxRowCountDirect` (limit reached inside the first frame) and `testMaxRowCountSplitOverTwoFramesDirect` (limit reached in a later frame, which is the case that used to hang). Local run on this branch, JDK 26 on macOS: ``` mvn test -pl sql -am -Dtest="org.apache.druid.sql.avatica.**" -Pskip-static-checks -Dweb.console.skip=true -T1C ``` `DruidStatementTest` 16/16 and `DruidAvaticaHandlerTest` 45/45 pass. `AvaticaModuleTest` and one case each in `DruidAvaticaJsonHandlerTest` and `DruidAvaticaProtobufHandlerTest` error with `Failed to mock class org.apache.druid.server.DruidNode`; those reproduce identically on an unmodified `master` checkout here, so they are a local JDK/EasyMock issue rather than a result of this change. `mvn checkstyle:check -pl sql` reports 0 violations. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
