From ef11e44adc252e048337a40bdc21d7cc6918a9f9 Mon Sep 17 00:00:00 2001 From: ningsh7 Date: Tue, 29 Sep 2026 09:19:36 +0800 Subject: [PATCH] feat(node): expose allowExternalBlobOutsideBases in Table.add (#4317) ## Summary - Add `allowExternalBlobOutsideBases` to the Node `AddDataOptions` and forward it through N-API to the Rust `AddDataBuilder`. - Document its default and local-only external blob behavior. - Cover URI strings and URI objects, option scoping, progress callbacks, overwrite mode, and remote rejection. ## Validation - `pnpm test --runInBand` (21 suites passed, 1 skipped; 908 tests passed, 6 skipped) - `pnpm lint-ci` - `pnpm tsc` - `pnpm run docs` - `cargo fmt --all -- --check` - `CC=clang cargo clippy --profile ci -p lancedb-nodejs --all-features -- -D warnings` Fixes #4267 --- docs/src/js/interfaces/AddDataOptions.md | 16 +++ nodejs/__test__/remote.test.ts | 63 +++++++++++ nodejs/__test__/table.test.ts | 127 +++++++++++++++++++++++ nodejs/lancedb/table.ts | 18 +++- nodejs/src/table.rs | 6 +- 5 files changed, 228 insertions(+), 2 deletions(-) diff --git a/docs/src/js/interfaces/AddDataOptions.md b/docs/src/js/interfaces/AddDataOptions.md index 6976787f0..eb10fdb0f 100644 --- a/docs/src/js/interfaces/AddDataOptions.md +++ b/docs/src/js/interfaces/AddDataOptions.md @@ -10,6 +10,22 @@ Options for adding data to a table. ## Properties +### allowExternalBlobOutsideBases? + +```ts +optional allowExternalBlobOutsideBases: boolean; +``` + +Whether blob URIs outside registered external bases may be written. + +Defaults to `false`. This option is supported only for local/native +tables; remote tables return an error when it is enabled. An enabled write +stores the absolute URI reference without registering a base or copying +the external object into the database. The object must remain accessible +when the blob is read later. + +*** + ### mode ```ts diff --git a/nodejs/__test__/remote.test.ts b/nodejs/__test__/remote.test.ts index cb0047f37..898ba533e 100644 --- a/nodejs/__test__/remote.test.ts +++ b/nodejs/__test__/remote.test.ts @@ -85,6 +85,69 @@ async function withMockDatabase( } describe("remote connection", () => { + it("rejects the external blob opt-in without blocking regular adds", async () => { + let insertRequests = 0; + const describeRequests: string[] = []; + + await withMockDatabase( + (req, res) => { + const requestPath = req.url ?? ""; + if (requestPath.endsWith("/describe/")) { + describeRequests.push(requestPath); + res.writeHead(200, { "Content-Type": "application/json" }).end( + JSON.stringify({ + name: "items", + version: 1, + schema: { + fields: [ + { + name: "id", + type: { type: "int64" }, + nullable: true, + }, + ], + }, + }), + ); + return; + } + if (requestPath.endsWith("/insert/")) { + insertRequests++; + req.resume(); + req.on("end", () => { + res + .writeHead(200, { "Content-Type": "application/json" }) + .end(JSON.stringify({ version: insertRequests + 1 })); + }); + return; + } + res.writeHead(404).end(); + }, + async (db) => { + const table = await db.openTable("items"); + + await expect( + table.add([{ id: 1n }], { + allowExternalBlobOutsideBases: true, + }), + ).rejects.toThrow("only supported on local tables"); + expect(insertRequests).toBe(0); + + await expect(table.add([{ id: 2n }])).resolves.toMatchObject({ + version: 2, + }); + await expect( + table.add([{ id: 3n }], { + allowExternalBlobOutsideBases: false, + }), + ).resolves.toMatchObject({ version: 3 }); + }, + ); + + expect(describeRequests.length).toBeGreaterThan(0); + expect(insertRequests).toBe(2); + }); + it("lists materialized views through the namespace route", async () => { await withMockDatabase( (req, res) => { diff --git a/nodejs/__test__/table.test.ts b/nodejs/__test__/table.test.ts index 7fe854438..ae257455d 100644 --- a/nodejs/__test__/table.test.ts +++ b/nodejs/__test__/table.test.ts @@ -2,6 +2,7 @@ // SPDX-FileCopyrightText: Copyright The LanceDB Authors import * as fs from "fs"; +import { pathToFileURL } from "node:url"; import * as path from "path"; import * as tmp from "tmp"; @@ -2449,6 +2450,118 @@ describe("when dealing with blob columns", () => { await expect(table.countRows()).resolves.toBe(0); }); + it("requires an explicit opt-in for blob URIs outside registered bases", async () => { + const { table, uri } = await openExternalBlobTable(); + + await expect(table.add([{ id: 1n, payload: uri }])).rejects.toThrow( + "allow_external_blob_outside_bases", + ); + await expect(table.countRows()).resolves.toBe(0); + + await expect( + table.add([{ id: 2n, payload: uri }], { + allowExternalBlobOutsideBases: false, + }), + ).rejects.toThrow("allow_external_blob_outside_bases"); + await expect(table.countRows()).resolves.toBe(0); + }); + + it.each([ + ["URI string", (uri: string) => uri], + ["URI struct", (uri: string) => ({ uri })], + ])( + "round-trips an external blob from a %s after reopening", + async (_label, blobValue) => { + const { db, table, uri, payload } = await openExternalBlobTable(); + + const result = await table.add([{ id: 1n, payload: blobValue(uri) }], { + allowExternalBlobOutsideBases: true, + }); + expect(result.version).toBeGreaterThan(0); + await expect(table.countRows()).resolves.toBe(1); + + table.close(); + const reopened = await db.openTable("external_blobs"); + const rows = await reopened.query().withRowId().toArray(); + expect(rows).toHaveLength(1); + expect(rows[0].payload.blob_uri).toBe(uri); + const bytes = await reopened.fetchBlobs("payload", [ + rows[0]._rowid as bigint, + ]); + expect(bytes[0]).toEqual(payload); + }, + ); + + it("applies the external blob opt-in to Arrow table input", async () => { + const { table, schema, uri, payload } = await openExternalBlobTable(); + const input = makeArrowTable([{ id: 1n, payload: uri }], { schema }); + + const result = await table.add(input, { + allowExternalBlobOutsideBases: true, + }); + + expect(result.version).toBeGreaterThan(0); + const rows = await table.query().withRowId().toArray(); + expect(rows[0].payload.blob_uri).toBe(uri); + const bytes = await table.fetchBlobs("payload", [rows[0]._rowid as bigint]); + expect(bytes[0]).toEqual(payload); + }); + + it("applies the external blob opt-in only to the current add", async () => { + const { table, uri } = await openExternalBlobTable(); + + await table.add([{ id: 1n, payload: uri }], { + allowExternalBlobOutsideBases: true, + }); + await expect(table.countRows()).resolves.toBe(1); + + await expect(table.add([{ id: 2n, payload: uri }])).rejects.toThrow( + "allow_external_blob_outside_bases", + ); + await expect(table.countRows()).resolves.toBe(1); + }); + + it("supports progress and overwrite with the external blob opt-in", async () => { + const { table, uri, payload } = await openExternalBlobTable(); + await table.add([{ id: 1n, payload: Buffer.from("old") }]); + + let resolveFinal!: (progress: import("../lancedb").WriteProgress) => void; + let rejectFinal!: (error: Error) => void; + const finalProgress = new Promise( + (resolve, reject) => { + resolveFinal = resolve; + rejectFinal = reject; + }, + ); + const timeout = setTimeout( + () => rejectFinal(new Error("timed out waiting for final progress")), + 5_000, + ); + try { + const result = await table.add([{ id: 2n, payload: uri }], { + mode: "overwrite", + allowExternalBlobOutsideBases: true, + progress: (progress) => { + if (progress.done) { + resolveFinal(progress); + } + }, + }); + expect(result.version).toBeGreaterThan(0); + const progress = await finalProgress; + expect(progress.outputRows).toBe(1); + expect(progress.totalRows).toBe(1); + } finally { + clearTimeout(timeout); + } + + const rows = await table.query().withRowId().toArray(); + expect(rows).toHaveLength(1); + expect(rows[0].id).toBe(2n); + const bytes = await table.fetchBlobs("payload", [rows[0]._rowid as bigint]); + expect(bytes[0]).toEqual(payload); + }); + it("discovers blob columns", async () => { const { table } = await openBlobTable(); expect(await table.blobColumns()).toEqual(["image"]); @@ -2708,6 +2821,20 @@ describe("when dealing with blob columns", () => { const rowIds = [1, 2, 3].map((id) => rowIdById.get(id)!); return { table, rowIds, alpha, beta }; } + + async function openExternalBlobTable() { + const db = await connect(path.join(tmpDir.name, "db")); + const payload = Buffer.from("external-payload"); + const payloadPath = path.join(tmpDir.name, "payload.bin"); + fs.writeFileSync(payloadPath, payload); + const uri = pathToFileURL(payloadPath).href; + const schema = new Schema([ + new Field("id", new Int64(), true), + blob("payload"), + ]); + const table = await db.createEmptyTable("external_blobs", schema); + return { db, table, schema, uri, payload }; + } }); describe("when dealing with tags", () => { diff --git a/nodejs/lancedb/table.ts b/nodejs/lancedb/table.ts index a098322a9..cabcf88aa 100644 --- a/nodejs/lancedb/table.ts +++ b/nodejs/lancedb/table.ts @@ -124,6 +124,17 @@ export interface AddDataOptions { * ``` */ progress: (progress: WriteProgress) => void; + + /** + * Whether blob URIs outside registered external bases may be written. + * + * Defaults to `false`. This option is supported only for local/native + * tables; remote tables return an error when it is enabled. An enabled write + * stores the absolute URI reference without registering a base or copying + * the external object into the database. The object must remain accessible + * when the blob is read later. + */ + allowExternalBlobOutsideBases?: boolean; } export interface UpdateOptions { @@ -1091,7 +1102,12 @@ export class LocalTable extends Table { } } : undefined; - return await this.inner.add(buffer, mode, progress); + return await this.inner.add( + buffer, + mode, + progress, + options?.allowExternalBlobOutsideBases, + ); } async update( diff --git a/nodejs/src/table.rs b/nodejs/src/table.rs index fa74c20dc..d381d98e5 100644 --- a/nodejs/src/table.rs +++ b/nodejs/src/table.rs @@ -74,13 +74,14 @@ impl Table { #[napi( catch_unwind, - ts_args_type = "buf: Buffer, mode: string, progressCallback?: (progress: WriteProgressInfo) => void" + ts_args_type = "buf: Buffer, mode: string, progressCallback?: (progress: WriteProgressInfo) => void, allowExternalBlobOutsideBases?: boolean" )] pub async fn add( &self, buf: Buffer, mode: String, progress_callback: Option, + allow_external_blob_outside_bases: Option, ) -> napi::Result { let batches = ipc_file_to_batches(buf.to_vec()) .map_err(|e| napi::Error::from_reason(format!("Failed to read IPC file: {}", e)))?; @@ -105,6 +106,9 @@ impl Table { return Err(napi::Error::from_reason(format!("Invalid mode: {}", mode))); }; + op = op + .allow_external_blob_outside_bases(allow_external_blob_outside_bases.unwrap_or(false)); + if let Some(tsfn) = progress_callback { op = op.progress(move |p| { // NonBlocking: dispatch onto the JS event loop without