From ca5a69ef399fe96aa9f17367e1bd63e589777056 Mon Sep 17 00:00:00 2001 From: Lee Houghton Date: Mon, 11 Mar 2024 15:49:30 +0000 Subject: [PATCH] NODEJS-668: Handle empty pages in async pagination This fixes an issue where an empty page in the results would cause pagination to end, even if there were more results to be returned. In practice this has happened when using `allow filtering` or when encountering deleted rows in Scylla materialised views. --- lib/types/result-set.js | 10 ++++++++-- test/unit/result-set-tests.js | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/types/result-set.js b/lib/types/result-set.js index 4137ddb03..a811931b0 100644 --- a/lib/types/result-set.js +++ b/lib/types/result-set.js @@ -233,7 +233,7 @@ ResultSet.prototype[asyncIteratorSymbol] = function getAsyncGenerator() { let pageState = this.rawPageState; let rows = this.rows; - if (!rows || rows.length === 0) { + if (!pageState && (!rows || rows.length === 0)) { return { next: () => Promise.resolve({ done: true }) }; } @@ -257,7 +257,13 @@ ResultSet.prototype[asyncIteratorSymbol] = function getAsyncGenerator() { return { done: false, value: rows[index++] }; } - return { done: true }; + if (!pageState) { + return { done: true }; + } + + // We could reach this point if an empty page is returned. + // An empty page doesn't necessarily mean the end of the results. + return this.next(); } }; }; diff --git a/test/unit/result-set-tests.js b/test/unit/result-set-tests.js index 26bb8a784..966d97fc5 100644 --- a/test/unit/result-set-tests.js +++ b/test/unit/result-set-tests.js @@ -135,6 +135,26 @@ describe('ResultSet', function () { assert.ok(rs.nextPageAsync.firstCall.calledWithExactly(pageState)); }); + it('should continue past empty pages', async () => { + const pageState = utils.allocBuffer(1); + const rs = new ResultSet( { rows: [ 100, 101, 102 ], meta: { pageState } }); + const pages = [ + [], + [], + [], + [ 103, 104, 105 ], + ]; + rs.nextPageAsync = sinon.spy(function () { + const rows = pages.shift(); + return Promise.resolve({ rows: rows, rawPageState: pages.length > 0 ? pageState : undefined }); + }); + + const result = await helper.asyncIteratorToArray(rs); + assert.deepEqual(result, [ 100, 101, 102, 103, 104, 105 ]); + assert.strictEqual(rs.nextPageAsync.callCount, 4); + assert.ok(rs.nextPageAsync.firstCall.calledWithExactly(pageState)); + }); + it('should reject when nextPageAsync rejects', async () => { const rs = new ResultSet( { rows: [ 100 ], meta: { pageState: utils.allocBuffer(1)} }); const error = new Error('Test dummy error');