Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

default duckdb_api config #59

Merged
merged 1 commit into from
Dec 12, 2024
+103 −16
Merged

default duckdb_api config #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions api/src/DuckDBInstance.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ export class DuckDBInstance {
path?: string,
options?: Record<string, string>
): Promise<DuckDBInstance> {
const config = duckdb.create_config();
// Set the default duckdb_api value for the api. Can be overridden.
duckdb.set_config(config, 'duckdb_api', 'node-neo-api');
if (options) {
const config = duckdb.create_config();
for (const optionName in options) {
const optionValue = String(options[optionName]);
duckdb.set_config(config, optionName, optionValue);
}
return new DuckDBInstance(await duckdb.open(path, config));
} else {
return new DuckDBInstance(await duckdb.open(path));
}
return new DuckDBInstance(await duckdb.open(path, config));
}
public async connect(): Promise<DuckDBConnection> {
return new DuckDBConnection(await duckdb.connect(this.db));
Expand Down
30 changes: 30 additions & 0 deletions api/test/api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,4 +973,34 @@ describe('api', () => {
assert.deepEqual(rows[4999], [4999, 14999]);
});
});
test('default duckdb_api without explicit options', async () => {
const instance = await DuckDBInstance.create();
const connection = await instance.connect();
const result = await connection.run(`select current_setting('duckdb_api') as duckdb_api`);
assertColumns(result, [{ name: 'duckdb_api', type: DuckDBVarCharType.instance }]);
const chunk = await result.fetchChunk();
assert.strictEqual(chunk.columnCount, 1);
assert.strictEqual(chunk.rowCount, 1);
assertValues<string, DuckDBVarCharVector>(chunk, 0, DuckDBVarCharVector, ['node-neo-api']);
});
test('default duckdb_api with explicit options', async () => {
const instance = await DuckDBInstance.create(undefined, {});
const connection = await instance.connect();
const result = await connection.run(`select current_setting('duckdb_api') as duckdb_api`);
assertColumns(result, [{ name: 'duckdb_api', type: DuckDBVarCharType.instance }]);
const chunk = await result.fetchChunk();
assert.strictEqual(chunk.columnCount, 1);
assert.strictEqual(chunk.rowCount, 1);
assertValues<string, DuckDBVarCharVector>(chunk, 0, DuckDBVarCharVector, ['node-neo-api']);
});
test('overriding duckdb_api', async () => {
const instance = await DuckDBInstance.create(undefined, { 'duckdb_api': 'custom-duckdb-api' });
const connection = await instance.connect();
const result = await connection.run(`select current_setting('duckdb_api') as duckdb_api`);
assertColumns(result, [{ name: 'duckdb_api', type: DuckDBVarCharType.instance }]);
const chunk = await result.fetchChunk();
assert.strictEqual(chunk.columnCount, 1);
assert.strictEqual(chunk.rowCount, 1);
assertValues<string, DuckDBVarCharVector>(chunk, 0, DuckDBVarCharVector, ['custom-duckdb-api']);
});
});
35 changes: 24 additions & 11 deletions bindings/src/duckdb_node_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

#include "duckdb.h"

#define DEFAULT_DUCKDB_API "node-neo-bindings"

// Conversion betweeen structs and objects

Napi::Object MakeDateObject(Napi::Env env, duckdb_date date) {
Expand Down Expand Up @@ -495,18 +497,25 @@ class OpenWorker : public PromiseWorker {
if (path_) {
path = path_->c_str();
}
if (config_ != nullptr) {
char *error = nullptr;
if (duckdb_open_ext(path, &database_, config_, &error)) {
if (error != nullptr) {
SetError(error);
duckdb_free(error);
} else {
SetError("Failed to open");
}
duckdb_config cfg = config_;
// If no config was provided, create one with the default duckdb_api value.
if (cfg == nullptr) {
if (duckdb_create_config(&cfg)) {
duckdb_destroy_config(&cfg);
SetError("Failed to create config");
return;
}
if (duckdb_set_config(cfg, "duckdb_api", DEFAULT_DUCKDB_API)) {
SetError("Failed to set duckdb_api");
return;
}
} else {
if (duckdb_open(path, &database_)) {
}
char *error = nullptr;
if (duckdb_open_ext(path, &database_, cfg, &error)) {
if (error != nullptr) {
SetError(error);
duckdb_free(error);
} else {
SetError("Failed to open");
}
}
Expand Down Expand Up @@ -1170,6 +1179,10 @@ class DuckDBNodeAddon : public Napi::Addon<DuckDBNodeAddon> {
duckdb_destroy_config(&config);
throw Napi::Error::New(env, "Failed to create config");
}
// Set the default duckdb_api value for the bindings. Can be overridden.
if (duckdb_set_config(config, "duckdb_api", DEFAULT_DUCKDB_API)) {
throw Napi::Error::New(env, "Failed to set duckdb_api");
}
return CreateExternalForConfig(env, config);
}

Expand Down
46 changes: 45 additions & 1 deletion bindings/test/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import duckdb from '@duckdb/node-bindings';
import { expect, suite, test } from 'vitest';
import { expectResult } from './utils/expectResult';
import { data } from './utils/expectedVectors';

suite('config', () => {
test('config_count', () => {
Expand All @@ -13,8 +15,50 @@ suite('config', () => {
expect(() => duckdb.get_config_flag(-1)).toThrowError(/^Config option not found$/);
});
test('create, set, and destroy', () => {
const config = duckdb.create_config()
const config = duckdb.create_config();
expect(config).toBeTruthy();
duckdb.set_config(config, 'custom_user_agent', 'my_user_agent');
});
test('default duckdb_api without explicit config', async () => {
const db = await duckdb.open();
const connection = await duckdb.connect(db);
const result = await duckdb.query(connection, `select current_setting('duckdb_api') as duckdb_api`);
await expectResult(result, {
columns: [
{ name: 'duckdb_api', logicalType: { typeId: duckdb.Type.VARCHAR } },
],
chunks: [
{ rowCount: 1, vectors: [data(16, [true], ['node-neo-bindings'])]},
],
});
});
test('default duckdb_api with explicit config', async () => {
const config = duckdb.create_config();
const db = await duckdb.open(undefined, config);
const connection = await duckdb.connect(db);
const result = await duckdb.query(connection, `select current_setting('duckdb_api') as duckdb_api`);
await expectResult(result, {
columns: [
{ name: 'duckdb_api', logicalType: { typeId: duckdb.Type.VARCHAR } },
],
chunks: [
{ rowCount: 1, vectors: [data(16, [true], ['node-neo-bindings'])]},
],
});
});
test('overriding duckdb_api', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overriding duckdb_api is not really expected (although possible). All layers above the closest-to-duckdb are expected to set custom_user_agent instead, which is additive (all layers will be preserved).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to make at least the bindings layer overridable, so the api layer can override it. But I agree it's unclear whether the ability to override this at the api level is useful, even though I support it for completeness.

const config = duckdb.create_config();
duckdb.set_config(config, 'duckdb_api', 'custom-duckdb-api');
const db = await duckdb.open(undefined, config);
const connection = await duckdb.connect(db);
const result = await duckdb.query(connection, `select current_setting('duckdb_api') as duckdb_api`);
await expectResult(result, {
columns: [
{ name: 'duckdb_api', logicalType: { typeId: duckdb.Type.VARCHAR } },
],
chunks: [
{ rowCount: 1, vectors: [data(16, [true], ['custom-duckdb-api'])]},
],
});
});
});
Loading