diff --git a/nodejs/__test__/connection.test.ts b/nodejs/__test__/connection.test.ts index e32e7e626..bbd0c3c52 100644 --- a/nodejs/__test__/connection.test.ts +++ b/nodejs/__test__/connection.test.ts @@ -336,6 +336,56 @@ describe("given a connection", () => { expect(seen).toEqual(created); }); + it.each([2 ** 32, 2 ** 32 + 1, Number.MAX_SAFE_INTEGER])( + "should not wrap a large table listing limit %s", + async (limit) => { + await db.createTable("a", [{ id: 1 }]); + await db.createTable("b", [{ id: 2 }]); + for (const names of [ + db.tableNames({ limit }), + db.tableNames([], { limit }), + ]) { + await expect(names).resolves.toEqual(["a", "b"]); + } + for (const page of [ + db.listTables({ limit }), + db.listTables([], { limit }), + ]) { + await expect(page).resolves.toEqual({ tables: ["a", "b"] }); + } + }, + ); + + it.each([ + -1, + 1.5, + Number.NaN, + Number.POSITIVE_INFINITY, + Number.NEGATIVE_INFINITY, + Number.MAX_SAFE_INTEGER + 1, + ])("should reject an invalid table listing limit %s", async (limit) => { + await expect(db.tableNames({ limit })).rejects.toThrow(/limit/); + await expect(db.tableNames([], { limit })).rejects.toThrow(/limit/); + await expect(db.listTables({ limit })).rejects.toThrow(/limit/); + await expect(db.listTables([], { limit })).rejects.toThrow(/limit/); + }); + + it("should preserve zero, small and unsigned-32-bit table listing limits", async () => { + await db.createTable("a", [{ id: 1 }]); + await db.createTable("b", [{ id: 2 }]); + for (const [limit, names] of [ + [0, []], + [1, ["a"]], + [2 ** 32 - 1, ["a", "b"]], + ] as const) { + await expect(db.tableNames({ limit })).resolves.toEqual(names); + const page = await db.listTables({ limit }); + expect(page.tables).toEqual(names); + } + await expect(db.tableNames()).resolves.toEqual(["a", "b"]); + await expect(db.listTables()).resolves.toEqual({ tables: ["a", "b"] }); + }); + it("should list all names and use a default page size of 100", async () => { const names = Array.from( { length: 130 }, diff --git a/nodejs/lancedb/connection.ts b/nodejs/lancedb/connection.ts index 44fde96f1..c93414fab 100644 --- a/nodejs/lancedb/connection.ts +++ b/nodejs/lancedb/connection.ts @@ -726,7 +726,7 @@ export class LocalConnection extends Connection { return this.inner.tableNames( namespacePath ?? [], tableNamesOptions?.startAfter, - tableNamesOptions?.limit, + normalizeTableListingLimit(tableNamesOptions?.limit), ); } @@ -836,7 +836,7 @@ export class LocalConnection extends Connection { return this.inner.listTables( namespacePath ?? [], listTablesOptions?.pageToken, - listTablesOptions?.limit, + normalizeTableListingLimit(listTablesOptions?.limit), ); } @@ -1107,6 +1107,13 @@ export class LocalConnection extends Connection { } } +function normalizeTableListingLimit(limit?: number): number | undefined { + validateNonNegativeInteger(limit, "limit"); + // The native binding takes u32: larger JS numbers otherwise wrap to zero or + // a smaller page size. A listing limit is an upper bound, so saturate it. + return limit === undefined ? undefined : Math.min(limit, 0xffffffff); +} + /** * Takes storage options and makes all the keys snake case. */