fix: fix off-by-one bug in ArrayResultSet.next() causing 'No current row' error#2
Merged
sophiecuiy merged 4 commits intomainfrom Mar 13, 2026
Merged
Conversation
…row' error
ArrayResultSet.next() returned true one extra time past the end of the
array. For an N-element array, next() was called N+1 times successfully
before getString(2) threw 'No current row' on the phantom iteration.
Root cause: the guard condition 'pos == length' checked BEFORE
incrementing pos, so when pos was at length-1 (last valid index), it
passed the guard, incremented to length, and returned true. The
subsequent checkRowPosition() then correctly detected pos >= length and
threw SQLException.
Fix: change the guard to 'pos >= length - 1' so that next() returns
false when pos is already at the last valid index.
The 'length == 0' guard is kept for clarity (empty arrays) even though
'pos >= length - 1' would handle it (pos=-1 >= -1 is true).
Affects all consumers that iterate ArrayResultSet with the standard
JDBC pattern: while (rs.next()) { rs.getString(2); }
Discovered via Airbyte connector source-clickhouse (airbytehq/airbyte#61419).
Co-Authored-By: sophie.cui@airbyte.io <sophie.cui@airbyte.io>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Collaborator
Matt Bayley (mwbayley)
left a comment
Matt Bayley (mwbayley)
left a comment
There was a problem hiding this comment.
Very close! Have you confirmed that the new tests fail without the change to next()? I imagine they do. I was going to suggest adding only 1 test, but now that I think about it, the cases are different. It's probably worth adding a test for the empty array case, too.
jdbc-v2/src/main/java/com/clickhouse/jdbc/types/ArrayResultSet.java
Outdated
Show resolved
Hide resolved
jdbc-v2/src/test/java/com/clickhouse/jdbc/types/ArrayResultSetTest.java
Outdated
Show resolved
Hide resolved
Removed comments explaining the test for next() method behavior.
Ensures next() returns false immediately for an empty array and continues returning false on subsequent calls. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ArrayResultSet.next()that caused it to returntrueone extra time past the end of the array, leading to a"No current row"SQLExceptionon the phantom iteration.pos == lengthwas checked before incrementingpos, so whenposwas atlength-1(last valid index), it passed the guard, incremented tolength, and returnedtrue. Fix: change topos >= length - 1.testNextIterationCount,testNextSingleElement) verifying the standard JDBC iteration patternwhile (rs.next()) { rs.getString(2); }works correctly.Discovered via Airbyte connector source-clickhouse (airbytehq/airbyte#61419).
Test plan
testNextIterationCountpasses — iterates a 3-element array exactly 3 timestestNextSingleElementpasses — single-element array iterates onceArrayResultSetTesttests still passjdbc-v2test suite🤖 Generated with Claude Code