-
Notifications
You must be signed in to change notification settings - Fork 35
Fixed ResultSetReader inconsistency #588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR standardizes ResultSetReader positioning/error semantics (before-first/after-last behavior, index validation, and error messages) and updates tests in both table and query modules to reflect the corrected behavior.
Changes:
- Tighten
ProtoResultSetReaderrow positioning semantics and add consistent bounds/state checks with stable exception messages. - Expand/adjust unit tests for column metadata, iteration, and truncated-result behavior.
- Update composite result set behavior in
QueryReadertests and implementation to usesetRowIndex(-1)as the reset-to-before-first convention.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| table/src/test/java/tech/ydb/table/result/ResultSetReaderTest.java | Adds comprehensive tests for column metadata, positioning rules, and truncation flag. |
| table/src/main/java/tech/ydb/table/result/impl/ProtoValueReaders.java | Deprecates forResultSets(...). |
| table/src/main/java/tech/ydb/table/result/impl/ProtoResultSetReader.java | Fixes positioning logic and adds consistent argument/state validation + messages. |
| table/src/main/java/tech/ydb/table/result/ResultSetReader.java | Updates/expands API documentation (needs minor corrections/clarification). |
| query/src/test/java/tech/ydb/query/tools/QueryReaderTest.java | Updates composite result set tests to match new positioning semantics. |
| query/src/main/java/tech/ydb/query/tools/QueryReader.java | Updates composite setRowIndex reset/position behavior (introduces edge-case bugs for empty/out-of-range). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release_v2.4.0 #588 +/- ##
====================================================
+ Coverage 65.16% 65.90% +0.73%
- Complexity 2734 2845 +111
====================================================
Files 344 344
Lines 14323 14412 +89
Branches 1481 1488 +7
====================================================
+ Hits 9334 9498 +164
+ Misses 4322 4252 -70
+ Partials 667 662 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.