Skip to content

Fix client-side maxRows enforcement in ResultSet.next()#1448

Open
gopalldb wants to merge 3 commits into
databricks:mainfrom
gopalldb:fix/client-side-maxrows-truncation
Open

Fix client-side maxRows enforcement in ResultSet.next()#1448
gopalldb wants to merge 3 commits into
databricks:mainfrom
gopalldb:fix/client-side-maxrows-truncation

Conversation

@gopalldb
Copy link
Copy Markdown
Collaborator

@gopalldb gopalldb commented May 15, 2026

Summary

Add client-side maxRows enforcement in DatabricksResultSet.next(). When statement.setMaxRows() is set, next() returns false once the row limit is reached, even if the server returns more rows. Applies uniformly to all result types (Thrift, SEA, inline, CloudFetch).

Problem

The server-side row_limit/resultRowLimit is not always honored — the server may return more rows than requested. Some IExecutionResult implementations (LazyThriftResult, StreamingColumnarResult, StreamingInlineArrowResult) enforce maxRows internally, but InlineJsonResult and ArrowStreamResult do not, leaving a gap.

Fix

Single enforcement point at DatabricksResultSet.next():

  • maxRowsLimit field cached from parentStatement.getMaxRows() at construction
  • rowsReturned counter incremented on each successful next()
  • Check maxRowsLimit > 0 && rowsReturned >= maxRowsLimit before delegating to executionResult.next()
  • maxRows == 0 means no limit (JDBC convention)
  • getUpdateCount() internal iteration bypasses the limit via countingUpdateRows flag

Existing per-implementation maxRows enforcement retained as defense-in-depth.

Test plan

  • testNextRespectsMaxRows — limit=3, 4th next() returns false
  • testNextMaxRowsZeroNoLimit — no truncation with limit=0
  • testNextMaxRowsNullParentNoLimit — null parent = no limit
  • testNextMaxRowsOneEdge — limit=1 edge case
  • testNextMaxRowsDoesNotCallExecutionResultAfterLimit — verify no unnecessary delegation
  • testNextMaxRowsWithEmptyResultSet — empty result + maxRows
  • testNextMaxRowsGreaterThanActualRows — natural exhaustion before limit
  • testNextMaxRowsIdempotenceAfterLimit — repeated calls after limit
  • testGetUpdateCountBypassesMaxRows — DML counting unaffected
  • Independent verification: 7/7 PASS (Agent C, black-box from spec)
  • Full suite: 62 tests pass

This pull request was AI-assisted by Isaac.

gopalldb added 3 commits May 15, 2026 15:53
Co-authored-by: Isaac
Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
Co-authored-by: Isaac
Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant