Skip to content

Commit 6c67e24

Browse files
sumimakitosumimakito
committed
fix: respect access mode and file open flags
1 parent 7f924a2 commit 6c67e24

File tree

10 files changed

+243
-36
lines changed

10 files changed

+243
-36
lines changed

‎packages/duckdb-wasm/src/bindings/bindings_base.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { DuckDBModule, PThread } from './duckdb_module';
2-
import { DuckDBConfig } from './config';
2+
import { DuckDBAccessMode, DuckDBConfig } from './config';
33
import { Logger } from '../log';
44
import { InstantiationProgress } from './progress';
55
import { DuckDBBindings } from './bindings_interface';
@@ -508,9 +508,9 @@ export abstract class DuckDBBindingsBase implements DuckDBBindings {
508508
}
509509
dropResponseBuffers(this.mod);
510510
}
511-
public async prepareFileHandle(fileName: string, protocol: DuckDBDataProtocol): Promise<void> {
511+
public async prepareFileHandle(fileName: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<void> {
512512
if (protocol === DuckDBDataProtocol.BROWSER_FSACCESS && this._runtime.prepareFileHandles) {
513-
const list = await this._runtime.prepareFileHandles([fileName], DuckDBDataProtocol.BROWSER_FSACCESS);
513+
const list = await this._runtime.prepareFileHandles([fileName], DuckDBDataProtocol.BROWSER_FSACCESS, accessMode);
514514
for (const item of list) {
515515
const { handle, path: filePath, fromCached } = item;
516516
if (!fromCached && handle.getSize()) {
@@ -522,9 +522,9 @@ export abstract class DuckDBBindingsBase implements DuckDBBindings {
522522
throw new Error(`prepareFileHandle: unsupported protocol ${protocol}`);
523523
}
524524
/** Prepare a file handle that could only be acquired aschronously */
525-
public async prepareDBFileHandle(path: string, protocol: DuckDBDataProtocol): Promise<void> {
525+
public async prepareDBFileHandle(path: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<void> {
526526
if (protocol === DuckDBDataProtocol.BROWSER_FSACCESS && this._runtime.prepareDBFileHandle) {
527-
const list = await this._runtime.prepareDBFileHandle(path, DuckDBDataProtocol.BROWSER_FSACCESS);
527+
const list = await this._runtime.prepareDBFileHandle(path, DuckDBDataProtocol.BROWSER_FSACCESS, accessMode);
528528
for (const item of list) {
529529
const { handle, path: filePath, fromCached } = item;
530530
if (!fromCached && handle.getSize()) {

‎packages/duckdb-wasm/src/bindings/bindings_interface.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { DuckDBConfig, DuckDBConnection, DuckDBDataProtocol, FileStatistics, InstantiationProgress } from '.';
1+
import { DuckDBAccessMode, DuckDBConfig, DuckDBConnection, DuckDBDataProtocol, FileStatistics, InstantiationProgress } from '.';
22
import { CSVInsertOptions, JSONInsertOptions, ArrowInsertOptions } from './insert_options';
33
import { ScriptTokens } from './tokens';
44
import { WebFile } from './web_file';
@@ -54,8 +54,8 @@ export interface DuckDBBindings {
5454
protocol: DuckDBDataProtocol,
5555
directIO: boolean,
5656
): Promise<HandleType>;
57-
prepareFileHandle(path: string, protocol: DuckDBDataProtocol): Promise<void>;
58-
prepareDBFileHandle(path: string, protocol: DuckDBDataProtocol): Promise<void>;
57+
prepareFileHandle(path: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<void>;
58+
prepareDBFileHandle(path: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<void>;
5959
globFiles(path: string): WebFile[];
6060
dropFile(name: string): void;
6161
dropFiles(): void;

‎packages/duckdb-wasm/src/bindings/runtime.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { DuckDBAccessMode } from './config';
12
import { DuckDBModule } from './duckdb_module';
23
import { UDFFunction } from './udf_function';
34
import * as udf_rt from './udf_runtime';
@@ -58,6 +59,16 @@ export enum FileFlags {
5859
FILE_FLAGS_FILE_CREATE_NEW = 1 << 4,
5960
//! Open file in append mode
6061
FILE_FLAGS_APPEND = 1 << 5,
62+
//! Open file with restrictive permissions (600 on linux/mac) can only be used when creating, throws if file exists
63+
FILE_FLAGS_PRIVATE = 1 << 6,
64+
//! Return NULL if the file does not exist instead of throwing an error
65+
FILE_FLAGS_NULL_IF_NOT_EXISTS = 1 << 7,
66+
//! Multiple threads may perform reads and writes in parallel
67+
FILE_FLAGS_PARALLEL_ACCESS = 1 << 8,
68+
//! Ensure that this call creates the file, throw is file exists
69+
FILE_FLAGS_EXCLUSIVE_CREATE = 1 << 9,
70+
//! Return NULL if the file exist instead of throwing an error
71+
FILE_FLAGS_NULL_IF_EXISTS = 1 << 10,
6172
}
6273

6374
/** Configuration for the AWS S3 Filesystem */
@@ -158,8 +169,8 @@ export interface DuckDBRuntime {
158169

159170
// Prepare a file handle that could only be acquired aschronously
160171
prepareFileHandle?: (path: string, protocol: DuckDBDataProtocol) => Promise<PreparedDBFileHandle[]>;
161-
prepareFileHandles?: (path: string[], protocol: DuckDBDataProtocol) => Promise<PreparedDBFileHandle[]>;
162-
prepareDBFileHandle?: (path: string, protocol: DuckDBDataProtocol) => Promise<PreparedDBFileHandle[]>;
172+
prepareFileHandles?: (path: string[], protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode) => Promise<PreparedDBFileHandle[]>;
173+
prepareDBFileHandle?: (path: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode) => Promise<PreparedDBFileHandle[]>;
163174

164175
// Internal API - experimental
165176
progressUpdate(final: number, percentage: number, iteration: number): void;

‎packages/duckdb-wasm/src/bindings/runtime_browser.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
} from './runtime';
1717
import { DuckDBModule } from './duckdb_module';
1818
import * as udf from './udf_runtime';
19+
import { DuckDBAccessMode } from './config';
1920

2021
const OPFS_PREFIX_LEN = 'opfs://'.length;
2122
const PATH_SEP_REGEX = /\/|\\/;
@@ -110,8 +111,11 @@ export const BROWSER_RUNTIME: DuckDBRuntime & {
110111
BROWSER_RUNTIME._opfsRoot = await navigator.storage.getDirectory();
111112
}
112113
},
113-
/** Prepare a file handle that could only be acquired aschronously */
114-
async prepareFileHandles(filePaths: string[], protocol: DuckDBDataProtocol): Promise<PreparedDBFileHandle[]> {
114+
/** Prepare a file handle that could only be acquired asynchronously */
115+
async prepareFileHandles(filePaths: string[], protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<PreparedDBFileHandle[]> {
116+
// DuckDBAccessMode.UNDEFINED will be treated as READ_WRITE
117+
// See: https://github.com/duckdb/duckdb/blob/5f5512b827df6397afd31daedb4bbdee76520019/src/main/database.cpp#L442-L444
118+
const isReadWrite = !accessMode || accessMode === DuckDBAccessMode.READ_WRITE;
115119
if (protocol === DuckDBDataProtocol.BROWSER_FSACCESS) {
116120
await BROWSER_RUNTIME.assignOPFSRoot();
117121
const prepare = async (path: string): Promise<PreparedDBFileHandle> => {
@@ -135,13 +139,16 @@ export const BROWSER_RUNTIME: DuckDBRuntime & {
135139
}
136140
// mkdir -p
137141
for (const folder of folders) {
138-
dirHandle = await dirHandle.getDirectoryHandle(folder, { create: true });
142+
dirHandle = await dirHandle.getDirectoryHandle(folder, { create: isReadWrite });
139143
}
140144
}
141145
const fileHandle = await dirHandle.getFileHandle(fileName, { create: false }).catch(e => {
142146
if (e?.name === 'NotFoundError') {
143-
console.debug(`File ${path} does not exists yet, creating...`);
144-
return dirHandle.getFileHandle(fileName, { create: true });
147+
if (isReadWrite) {
148+
console.debug(`File ${path} does not exists yet, creating...`);
149+
return dirHandle.getFileHandle(fileName, { create: true });
150+
}
151+
console.debug(`File ${path} does not exists, aborting as we are in read-only mode`);
145152
}
146153
throw e;
147154
});
@@ -166,11 +173,11 @@ export const BROWSER_RUNTIME: DuckDBRuntime & {
166173
}
167174
throw new Error(`Unsupported protocol ${protocol} for paths ${filePaths} with protocol ${protocol}`);
168175
},
169-
/** Prepare a file handle that could only be acquired aschronously */
170-
async prepareDBFileHandle(dbPath: string, protocol: DuckDBDataProtocol): Promise<PreparedDBFileHandle[]> {
176+
/** Prepare a file handle that could only be acquired asynchronously */
177+
async prepareDBFileHandle(dbPath: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<PreparedDBFileHandle[]> {
171178
if (protocol === DuckDBDataProtocol.BROWSER_FSACCESS && this.prepareFileHandles) {
172179
const filePaths = [dbPath, `${dbPath}.wal`];
173-
return this.prepareFileHandles(filePaths, protocol);
180+
return this.prepareFileHandles(filePaths, protocol, accessMode);
174181
}
175182
throw new Error(`Unsupported protocol ${protocol} for path ${dbPath} with protocol ${protocol}`);
176183
},

‎packages/duckdb-wasm/src/bindings/runtime_node.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,32 @@ export const NODE_RUNTIME: DuckDBRuntime & {
7474
switch (file?.dataProtocol) {
7575
// Native file
7676
case DuckDBDataProtocol.NODE_FS: {
77+
let openFlags = fs.constants.O_RDONLY;
78+
if (flags & FileFlags.FILE_FLAGS_WRITE) {
79+
openFlags = fs.constants.O_RDWR;
80+
}
81+
if (flags & FileFlags.FILE_FLAGS_FILE_CREATE) {
82+
openFlags |= fs.constants.O_CREAT;
83+
} else if (flags & FileFlags.FILE_FLAGS_FILE_CREATE_NEW) {
84+
openFlags |= fs.constants.O_TRUNC;
85+
}
7786
let fd = NODE_RUNTIME._files?.get(file.dataUrl!);
78-
if (fd === null || fd === undefined) {
79-
fd = fs.openSync(
80-
file.dataUrl!,
81-
fs.constants.O_CREAT | fs.constants.O_RDWR,
82-
fs.constants.S_IRUSR | fs.constants.S_IWUSR,
83-
);
84-
NODE_RUNTIME._filesById?.set(file.fileId!, fd);
87+
let fileSize = 0;
88+
try {
89+
if (fd === null || fd === undefined) {
90+
fd = fs.openSync(file.dataUrl!, openFlags, fs.constants.S_IRUSR | fs.constants.S_IWUSR);
91+
NODE_RUNTIME._filesById?.set(file.fileId!, fd);
92+
}
93+
fileSize = fs.fstatSync(fd).size;
94+
}
95+
catch (e: any) {
96+
if (e.code === 'ENOENT' && (flags & FileFlags.FILE_FLAGS_NULL_IF_NOT_EXISTS)) {
97+
// No-op because we intend to ignore ENOENT while the file does not exist
98+
return 0; // nullptr
99+
} else {
100+
throw e;
101+
}
85102
}
86-
const fileSize = fs.fstatSync(fd).size;
87103
const result = mod._malloc(2 * 8);
88104
mod.HEAPF64[(result >> 3) + 0] = +fileSize;
89105
mod.HEAPF64[(result >> 3) + 1] = 0;

‎packages/duckdb-wasm/src/parallel/worker_dispatcher.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,9 @@ export abstract class AsyncDuckDBDispatcher implements Logger {
136136

137137
case WorkerRequestType.OPEN: {
138138
const path = request.data.path;
139+
const accessMode = request.data.accessMode;
139140
if (path?.startsWith('opfs://')) {
140-
await this._bindings.prepareDBFileHandle(path, DuckDBDataProtocol.BROWSER_FSACCESS);
141+
await this._bindings.prepareDBFileHandle(path, DuckDBDataProtocol.BROWSER_FSACCESS, accessMode);
141142
request.data.useDirectIO = true;
142143
}
143144
this._bindings.open(request.data);

‎packages/duckdb-wasm/test/index_node.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ import { testAllTypes, testAllTypesAsync } from './all_types.test';
6969
import { testBindings, testAsyncBindings } from './bindings.test';
7070
import { testBatchStream } from './batch_stream.test';
7171
import { testFilesystem } from './filesystem.test';
72+
import { testNodeFS } from './nodefs.test';
7273
import { testAsyncBatchStream } from './batch_stream_async.test';
7374
import { testArrowInsert, testArrowInsertAsync } from './insert_arrow.test';
7475
import { testJSONInsert, testJSONInsertAsync } from './insert_json.test';
@@ -92,6 +93,7 @@ testAsyncBindings(() => adb!, dataDir, duckdb.DuckDBDataProtocol.NODE_FS);
9293
testBatchStream(() => db!);
9394
testAsyncBatchStream(() => adb!);
9495
testFilesystem(() => adb!, resolveData, dataDir, duckdb.DuckDBDataProtocol.NODE_FS);
96+
testNodeFS(() => adb!);
9597
testArrowInsert(() => db!);
9698
testArrowInsertAsync(() => adb!);
9799
testJSONInsert(() => db!);

‎packages/duckdb-wasm/test/nodefs.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import * as duckdb from '../src/';
2+
import { tmpdir } from 'os';
3+
import { randomUUID } from 'crypto';
4+
import path from 'path';
5+
import { unlink } from 'fs/promises';
6+
7+
export function testNodeFS(db: () => duckdb.AsyncDuckDB): void {
8+
const files: string[] = [];
9+
10+
afterAll(async () => {
11+
await Promise.all(files.map(file => unlink(file).catch(() => {})));
12+
await db().flushFiles();
13+
await db().dropFiles();
14+
});
15+
16+
describe('Node FS', () => {
17+
it('Should not create an empty DB file in read-only mode for non-existent path', async () => {
18+
const tmp = tmpdir();
19+
const filename = `duckdb_test_${randomUUID().replace(/-/g, '')}`;
20+
files.push(path.join(tmp, filename));
21+
22+
await expectAsync(
23+
db().open({
24+
path: path.join(tmp, filename),
25+
accessMode: duckdb.DuckDBAccessMode.READ_ONLY,
26+
}),
27+
).toBeRejectedWithError(/database does not exist/);
28+
});
29+
30+
it('Should create DB file in read-write mode for non-existent path', async () => {
31+
const tmp = tmpdir();
32+
const filename = `duckdb_test_${randomUUID().replace(/-/g, '')}`;
33+
files.push(path.join(tmp, filename));
34+
35+
await expectAsync(
36+
db().open({
37+
path: path.join(tmp, filename),
38+
accessMode: duckdb.DuckDBAccessMode.READ_WRITE,
39+
}),
40+
).toBeResolved();
41+
});
42+
43+
it('Should create an empty DB file in read-only mode for non-existent path with direct I/O', async () => {
44+
const tmp = tmpdir();
45+
const filename = `duckdb_test_${randomUUID().replace(/-/g, '')}`;
46+
files.push(path.join(tmp, filename));
47+
48+
await expectAsync(
49+
db().open({
50+
path: path.join(tmp, filename),
51+
accessMode: duckdb.DuckDBAccessMode.READ_ONLY,
52+
useDirectIO: true,
53+
}),
54+
).toBeRejectedWithError(/database does not exist/);
55+
});
56+
57+
it('Should create DB file in read-write mode for non-existent path with direct I/O', async () => {
58+
const tmp = tmpdir();
59+
const filename = `duckdb_test_${randomUUID().replace(/-/g, '')}`;
60+
files.push(path.join(tmp, filename));
61+
62+
await expectAsync(
63+
db().open({
64+
path: path.join(tmp, filename),
65+
accessMode: duckdb.DuckDBAccessMode.READ_WRITE,
66+
useDirectIO: true,
67+
}),
68+
).toBeResolved();
69+
});
70+
});
71+
}

‎packages/duckdb-wasm/test/opfs.test.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,85 @@ export function testOPFS(baseDir: string, bundle: () => duckdb.DuckDBBundle): vo
276276
});
277277
});
278278

279+
describe('Open database in OPFS', () => {
280+
it('should not open a non-existent DB file in read-only', async () => {
281+
const logger = new duckdb.ConsoleLogger(LogLevel.ERROR);
282+
const worker = new Worker(bundle().mainWorker!);
283+
const db_ = new duckdb.AsyncDuckDB(logger, worker);
284+
await db_.instantiate(bundle().mainModule, bundle().pthreadWorker);
285+
286+
await expectAsync(db_.open({
287+
path: 'opfs://non_existent.db',
288+
accessMode: duckdb.DuckDBAccessMode.READ_ONLY,
289+
})).toBeRejectedWithError(Error, /file or directory could not be found/);
290+
291+
await db_.terminate();
292+
await worker.terminate();
293+
294+
// Files should not be found with DuckDBAccessMode.READ_ONLY
295+
const opfsRoot = await navigator.storage.getDirectory();
296+
await expectAsync(opfsRoot.getFileHandle('non_existent.db', { create: false }))
297+
.toBeRejectedWithError(Error, /file or directory could not be found/);
298+
});
299+
300+
it('should not open a non-existent DB file and mkdir in read-only', async () => {
301+
const logger = new duckdb.ConsoleLogger(LogLevel.ERROR);
302+
const worker = new Worker(bundle().mainWorker!);
303+
const db_ = new duckdb.AsyncDuckDB(logger, worker);
304+
await db_.instantiate(bundle().mainModule, bundle().pthreadWorker);
305+
306+
await expectAsync(db_.open({
307+
path: 'opfs://duckdb_test/path/to/non_existent.db',
308+
accessMode: duckdb.DuckDBAccessMode.READ_ONLY,
309+
})).toBeRejectedWithError(Error, /file or directory could not be found/);
310+
311+
await db_.terminate();
312+
await worker.terminate();
313+
});
314+
315+
it('should open a non-existent DB file and mkdir in read-write', async () => {
316+
const logger = new duckdb.ConsoleLogger(LogLevel.ERROR);
317+
const worker = new Worker(bundle().mainWorker!);
318+
const db_ = new duckdb.AsyncDuckDB(logger, worker);
319+
await db_.instantiate(bundle().mainModule, bundle().pthreadWorker);
320+
321+
await expectAsync(db_.open({
322+
path: 'opfs://duckdb_test/path/to/duck.db',
323+
accessMode: duckdb.DuckDBAccessMode.READ_WRITE,
324+
})).toBeResolved();
325+
326+
await db_.terminate();
327+
await worker.terminate();
328+
});
329+
330+
it('should open a non-existent DB file in read-write and create files', async () => {
331+
const logger = new duckdb.ConsoleLogger(LogLevel.ERROR);
332+
const worker = new Worker(bundle().mainWorker!);
333+
const db_ = new duckdb.AsyncDuckDB(logger, worker);
334+
await db_.instantiate(bundle().mainModule, bundle().pthreadWorker);
335+
336+
const opfsRoot = await navigator.storage.getDirectory();
337+
338+
// Ensure files do not exist
339+
await expectAsync(opfsRoot.getFileHandle('non_existent_2.db', { create: false }))
340+
.toBeRejectedWithError(Error, /file or directory could not be found/);
341+
await expectAsync(opfsRoot.getFileHandle('non_existent_2.db.wal', { create: false }))
342+
.toBeRejectedWithError(Error, /file or directory could not be found/);
343+
344+
await expectAsync(db_.open({
345+
path: 'opfs://non_existent_2.db',
346+
accessMode: duckdb.DuckDBAccessMode.READ_WRITE,
347+
})).toBeResolved();
348+
349+
await db_.terminate();
350+
await worker.terminate();
351+
352+
// Files should be found with DuckDBAccessMode.READ_WRITE
353+
await expectAsync(opfsRoot.getFileHandle('non_existent_2.db', { create: false })).toBeResolved();
354+
await expectAsync(opfsRoot.getFileHandle('non_existent_2.db.wal', { create: false })).toBeResolved();
355+
});
356+
})
357+
279358
async function removeFiles() {
280359
const opfsRoot = await navigator.storage.getDirectory();
281360
await opfsRoot.removeEntry('test.db').catch(() => {});
@@ -292,5 +371,12 @@ export function testOPFS(baseDir: string, bundle: () => duckdb.DuckDBBundle): vo
292371
//
293372
}
294373
await opfsRoot.removeEntry('datadir').catch(() => {});
374+
375+
// In case of failure caused leftovers
376+
await opfsRoot.removeEntry('non_existent.db').catch(() => {});
377+
await opfsRoot.removeEntry('non_existent.db.wal').catch(() => {});
378+
await opfsRoot.removeEntry('non_existent_2.db').catch(() => {});
379+
await opfsRoot.removeEntry('non_existent_2.db.wal').catch(() => {});
380+
await opfsRoot.removeEntry('duckdb_test', { recursive: true }).catch(() => {});
295381
}
296382
}

0 commit comments

Comments
 (0)