Copilot commented on code in PR #10:
URL:
https://github.com/apache/ignite-nodejs-thin-client/pull/10#discussion_r3371018099
##########
src/Cursor.ts:
##########
@@ -27,7 +26,7 @@ import {CacheEntry} from "./CacheClient";
export abstract class BaseCursor<T> {
- protected _id: Long;
+ protected _id: any;
Review Comment:
Changing `_id` from `Long` to `any` removes type-safety in the generated
`.d.ts` and makes it easier to accidentally pass non-Long values into
`_write()` / cursor-close requests. Since the code already reads/writes cursor
IDs as `Long` values, keep the `Long` type and add a type-only import instead
of using `any`.
##########
src/Cursor.ts:
##########
@@ -74,8 +73,13 @@ export abstract class BaseCursor<T> {
* @return {boolean} - true if more cache entries are available, false
otherwise.
*/
hasMore(): boolean {
+ // _buffer is set on a freshly-opened cursor and holds the first page
of
+ // results before _getValues() has been called for the first time.
+ // Without this check, hasMore() incorrectly returns false on a new
cursor
+ // because _hasNext starts as false and _values starts as null.
return this._hasNext ||
- this._values && this._valueIndex < this._values.length;
+ (this._values != null && this._valueIndex < this._values.length) ||
+ this._buffer != null;
Review Comment:
`hasMore()` now returns `true` whenever `_buffer` is non-null, even if the
first (buffered) page contains `rowCount = 0` and `hasNext = false` (i.e. a
query returning zero rows). In that case callers may see `hasMore() === true`
but `getValue()` returns `null`, which violates the method contract (“more
cache entries are available”). Consider peeking the buffered page’s `rowCount`
(and the trailing `hasNext` boolean when `rowCount === 0`) without consuming
it, instead of treating any non-null buffer as “more entries”.
--
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]
