Repository navigation
fix(jdbc): match xerial result set navigation boundaries - #72
Merged
Merged
Conversation
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
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.
Closes #45. Part of #54.
What changed
LibsqlResultSet.findColumnnow returns the first match for duplicate labels (case-insensitive), matching xerial, and reports unknown labels asno such column: '<label>'.next()read the first row instead of throwing.null, numeric getters return0, andgetBooleanreturnsfalse. Typed numericgetObject(Class)still throwsBad value for type X, as xerial does.getRow()returns0before the first row, the 1-based current row while positioned, and the last row number after the end, including after repeatednext()calls.isLast()now always throwsSQLFeatureNotSupportedException("not supported by sqlite").wasNull()now tracks the last column read and the current cursor position, including xerial's boundary exceptions (see the table below).LibsqlResultSetNavigationTestcovering duplicate labels and every cursor boundary in the acceptance criteria.Contract decision
Parity with xerial 3.53.4.0 at the pinned tag, as required by #54. Every claim was re-probed against the published
org.xerial:sqlite-jdbc:3.53.4.0jar (and cross-checked with the vendored tag sources) before pinning tests.findColumnSQLException("no such column: '<label>'").next()next()).nullfor text/object/bytes/decimal/date getters,0for numeric getters,falseforgetBoolean; typed numericgetObject(Class)throwsBad value for type X.getRow0before the first row, 1-based current row while positioned, and the last row number after the end (stays there on repeatednext()returns offalse).isLastSQLFeatureNotSupportedException("not supported by sqlite").wasNullSQLExceptionwhile no column has been read in the current row:column 0 out of bounds [1,N]before the firstnext(), andcolumn -1 out of bounds [1,N]immediately after transitioning past the last row. After a getter at the past-end position it returnstrue, because the pointer reports SQL NULL.Probe corrections to the issue body:
wasNullbefore any getter is notfalse; it throws. OnlygetRowandgetInthad been re-verified in #28, so the duplicate-label,isLast, andwasNullclaims were probed again here.Evidence
Failing before the fix, on the current code (
./mvnw -pl libsql-jdbc -am -Dtest=LibsqlResultSetNavigationTest -Dsurefire.failIfNoSpecifiedTests=false test):Passing after the fix:
The conformance run included the selected xerial
ResultSetTest(21 tests) against sqld.Deviations
None. The change is pure parity with xerial 3.53.4.0; the Hrana transport does not prevent any of these semantics.
Out-of-scope defect noticed
isFirst()still differs from xerial after the end of a single-row result. xerial returnstruethere because its internal row counter stays at the last row; this implementation returnsfalse. That method is not one of the six navigation methods in this issue's contract and was left unchanged. LikewiseisBeforeFirst/isAfterLastboundary semantics were left as-is; they already matched the probed behavior for the cases covered.