-
Notifications
You must be signed in to change notification settings - Fork 50
feat: add mariadb connector
#187
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| --- | ||
| icon: simple-icons:mariadb | ||
| --- | ||
|
|
||
| # MariaDB | ||
|
|
||
| > Connect DB0 to MariaDB Database using mariadb | ||
|
|
||
| ## Usage | ||
|
|
||
| For this connector, you need to install [`mariadb`](https://www.npmjs.com/package/mariadb) dependency: | ||
|
|
||
| :pm-install{name="mariadb"} | ||
|
|
||
| Use `mariadb` connector: | ||
|
|
||
| ```js | ||
| import { createDatabase } from "db0"; | ||
| import mariadb from "db0/connectors/mariadb"; | ||
|
|
||
| const db = createDatabase( | ||
| mariadb({ | ||
| /* options */ | ||
| }), | ||
| ); | ||
| ``` | ||
|
|
||
| ## Options | ||
|
|
||
| :read-more{to="https://github.com/mariadb-corporation/mariadb-connector-nodejs/blob/main/types/share.d.ts#L603"} |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import mariadb from "mariadb"; | ||
| import type { Connector, Primitive } from "db0"; | ||
| import { BoundableStatement } from "./_internal/statement.ts"; | ||
|
|
||
| export type ConnectorOptions = mariadb.ConnectionConfig; | ||
|
|
||
| type InternalQuery = ( | ||
| sql: string, | ||
| params?: unknown[], | ||
| ) => Promise<mariadb.UpsertResult | Record<string, unknown>[]>; | ||
|
|
||
| export default function mariadbConnector( | ||
| opts: ConnectorOptions, | ||
| ): Connector<mariadb.Connection> { | ||
| let _connection: mariadb.Connection | undefined; | ||
| const getConnection = async () => { | ||
| if (_connection) { | ||
| return _connection; | ||
| } | ||
|
|
||
| _connection = await mariadb.createConnection({ | ||
| ...opts, | ||
| }); | ||
|
|
||
| return _connection; | ||
| }; | ||
|
|
||
| const query: InternalQuery = (sql, params) => | ||
| getConnection().then((c) => c.query(sql, params)); | ||
|
|
||
| return { | ||
| name: "mariadb", | ||
| dialect: "mariadb", | ||
| getInstance: () => getConnection(), | ||
| exec: (sql) => query(sql), | ||
| prepare: (sql) => new StatementWrapper(sql, query), | ||
| dispose: async () => { | ||
| await _connection?.end?.(); | ||
| _connection = undefined; | ||
| }, | ||
|
Comment on lines
+15
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Memoize the first Line 21 assigns 🔒 Proposed fix for connection initialization export default function mariadbConnector(
opts: ConnectorOptions,
): Connector<mariadb.Connection> {
let _connection: mariadb.Connection | undefined;
+ let _connectionPromise: Promise<mariadb.Connection> | undefined;
+
const getConnection = async () => {
if (_connection) {
return _connection;
}
+ if (_connectionPromise) {
+ return _connectionPromise;
+ }
- _connection = await mariadb.createConnection({
- ...opts,
- });
+ _connectionPromise = mariadb.createConnection({ ...opts }).then((connection) => {
+ _connection = connection;
+ return connection;
+ }).catch((error) => {
+ _connectionPromise = undefined;
+ throw error;
+ });
- return _connection;
+ return _connectionPromise;
};
const query: InternalQuery = (sql, params) =>
getConnection().then((c) => c.query(sql, params));
@@
exec: (sql) => query(sql),
prepare: (sql) => new StatementWrapper(sql, query),
dispose: async () => {
- await _connection?.end?.();
+ const connection = _connection ?? (await _connectionPromise);
+ await connection?.end?.();
_connection = undefined;
+ _connectionPromise = undefined;
},
};
}🤖 Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same lazy-init pattern used across all connectors here (mysql2, postgresql, etc). Fixing the race would be a separate change touching every connector, not scoped to this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: Length of output: 3883
Would you like me to open a follow-up GitHub issue to track fixing the promise-memoization race across all affected connectors ( 🐇 ✏️ Learnings added
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pi0 FYI, to be verified for race condition. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| }; | ||
| } | ||
|
|
||
| class StatementWrapper extends BoundableStatement<void> { | ||
| #query: InternalQuery; | ||
| #sql: string; | ||
|
|
||
| constructor(sql: string, query: InternalQuery) { | ||
| super(); | ||
| this.#sql = sql; | ||
| this.#query = query; | ||
| } | ||
|
|
||
| async all(...params: Primitive[]) { | ||
| const res = await this.#query(this.#sql, params); | ||
| return res as Record<string, unknown>[]; | ||
| } | ||
|
|
||
| async run(...params: Primitive[]) { | ||
| const res = (await this.#query(this.#sql, params)) as mariadb.UpsertResult; | ||
| return { | ||
| success: true, | ||
| ...res, | ||
| }; | ||
| } | ||
|
|
||
| async get(...params: Primitive[]) { | ||
| const res = await this.#query(this.#sql, params); | ||
| return (res as Record<string, unknown>[])[0]; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,16 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { describe } from "vitest"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import connector from "../../src/connectors/mariadb"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { testConnector } from "./_tests"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe.runIf(process.env.MARIADB_URL)("connectors: mariadb.test", () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| testConnector({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dialect: "mariadb", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| connector: connector({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| host: "localhost", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| port: 3307, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user: "test", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| password: "test", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| database: "db0", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+5
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use Line 5 gates the suite on 🔧 Proposed test setup import { describe } from "vitest";
import connector from "../../src/connectors/mariadb";
import { testConnector } from "./_tests";
-describe.runIf(process.env.MARIADB_URL)("connectors: mariadb.test", () => {
+const mariadbUrl = process.env.MARIADB_URL;
+
+describe.runIf(Boolean(mariadbUrl))("connectors: mariadb.test", () => {
+ const url = new URL(mariadbUrl!);
+
testConnector({
dialect: "mariadb",
connector: connector({
- host: "localhost",
- port: 3307,
- user: "test",
- password: "test",
- database: "db0",
+ host: url.hostname,
+ port: Number(url.port || 3306),
+ user: decodeURIComponent(url.username),
+ password: decodeURIComponent(url.password),
+ database: url.pathname.replace(/^\//, ""),
}),
});
});📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Matches the existing pattern - mysql2.test.ts does the same thing. The env var is just a gate, not the actual DSN. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: Length of output: 380
🧠 Learnings used |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.