Skip to content

feat: use sql params #14

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

Merged
killagu merged 4 commits into from
Nov 25, 2025
Merged

feat: use sql params #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8fa81bf
feat: use sql params
killagu Nov 25, 2025
df0ba71
Update src/types.ts
killagu Nov 25, 2025
45ce6ac
f
killagu Nov 25, 2025
e820e8a
f
killagu Nov 25, 2025
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
2 changes: 2 additions & 0 deletions src/channels.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ export interface ConnectionEnqueueMessage {

export interface QueryStartMessage {
connection: PoolConnectionPromisify;
values?: object | any[];
Copy link

@coderabbitai coderabbitai bot Nov 25, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Type mismatch with PoolConnectionPromisify.query interface.

The values type here is object | any[], but in types.ts the query method accepts any | any[] | { [param: string]: any }, which includes primitives. This inconsistency could cause type errors when forwarding query parameters to these message interfaces.

Consider aligning the types. Apply this diff to match the types.ts interface:

 export interface QueryStartMessage {
   connection: PoolConnectionPromisify;
-  values?: object | any[];
+  values?: any | any[] | { [param: string]: any };
   sql: string;
 }
 
 export interface QueryEndMessage {
   connection: PoolConnectionPromisify;
   sql: string;
-  values?: object | any[];
+  values?: any | any[] | { [param: string]: any };
   duration: number;
   error?: Error;
 }

Alternatively, verify whether MySQL actually accepts primitive values for query parameters, and if not, update types.ts to use the stricter type:

Also applies to: 34-34

🏁 Script executed:

#!/bin/bash
# Search for how values are used with the MySQL connection
rg -n -C3 -t ts 'conn\.query\(' src/

Length of output: 567

[/run_scripts]

🏁 Script executed:

#!/bin/bash
# Search for types.ts and look for query method/PoolConnectionPromisify definition
rg -n 'PoolConnectionPromisify|query.*values' src/ --type ts -A2 -B2 | head -100

Length of output: 4458

🏁 Script executed:

#!/bin/bash
# Find and examine types.ts
fd 'types\.ts$' src/ -exec cat {} \;

Length of output: 146

🏁 Script executed:

#!/bin/bash
# Search for where PoolConnectionPromisify.query is actually defined
rg -n 'query\s*\(' src/ --type ts -B5 | grep -A5 -B5 'PoolConnectionPromisify\|interface.*Connection'

Length of output: 440

[/run_scripts]

🏁 Script executed:

#!/bin/bash
# View channels.ts to see the current state
cat src/channels.ts

Length of output: 1130

🏁 Script executed:

#!/bin/bash
# Check if values is ever passed as a primitive (non-object/non-array) type in the codebase
rg -n 'values\s*=\s*[^[\{\s]|values\s*:\s*(?!object|any\[|\{)' src/ --type ts | head -20

Length of output: 129

Type mismatch confirmed between PoolConnectionPromisify.query interface and usage.

The type inconsistency is real and documented:

  • types.ts declares: query(sql: string, values?: any | any[] | { [param: string]: any }): Promise<any>;
  • channels.ts (lines 27, 34): values?: object | any[];
  • All implementations (operator.ts, connection.ts, transaction.ts): values?: object | any[]

The broader type in types.ts (which includes primitives via any) conflicts with the narrower type in channels.ts and all implementations (object | any[]). This inconsistency could cause type errors if a caller passes a primitive value to the query method.

However, note that all implementations consistently use the narrower type, not the broader one declared in types.ts. Before applying the suggested diff, verify which direction to align: either make channels.ts match types.ts (accepting the broader type), or update types.ts to match all implementations and channel messages (using the narrower type).

🤖 Prompt for AI Agents 
In src/channels.ts around lines 27 and 34, there's a mismatch between the
narrower values type used by channels and implementations (values?: object |
any[]) and the broader signature in types.ts that allows primitives; update the
signature in types.ts to match the implementations by changing the query
parameter type to values?: object | any[] (or equivalent indexable object type
if preferred), then run TypeScript type-checks and adjust any remaining
declarations so all interfaces and implementations use the same narrower type.

sql: string;
}

export interface QueryEndMessage {
connection: PoolConnectionPromisify;
sql: string;
values?: object | any[];
duration: number;
error?: Error;
}
4 changes: 2 additions & 2 deletions src/connection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ export class RDSConnection extends Operator {
return this.conn.release();
}

async _query(sql: string) {
return await this.conn.query(sql);
async _query(sql: string, values?: object | any[]) {
return await this.conn.query(sql, values);
}

async beginTransaction() {
Expand Down
13 changes: 6 additions & 7 deletions src/operator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,9 @@ export abstract class Operator {

async query<T = any>(sql: string, values?: object | any[]): Promise<T> {
// query(sql, values)
if (values) {
sql = this.format(sql, values);
}
if (this.beforeQueryHandlers.length > 0) {
for (const beforeQueryHandler of this.beforeQueryHandlers) {
const newSql = beforeQueryHandler(sql);
const newSql = beforeQueryHandler(sql, values);
if (newSql) {
sql = newSql;
}
Expand All @@ -95,10 +92,11 @@ export abstract class Operator {
let lastError: Error | undefined;
channels.queryStart.publish({
sql,
values,
connection: this.#connection,
} as QueryStartMessage);
try {
rows = await this._query(sql);
rows = await this._query(sql, values);
if (Array.isArray(rows)) {
debug('[connection#%s] query get %o rows', this.threadId, rows.length);
} else {
Expand All @@ -115,12 +113,13 @@ export abstract class Operator {
channels.queryEnd.publish({
sql,
connection: this.#connection,
values,
duration,
error: lastError,
} as QueryEndMessage);
if (this.afterQueryHandlers.length > 0) {
for (const afterQueryHandler of this.afterQueryHandlers) {
afterQueryHandler(sql, rows, duration, lastError);
afterQueryHandler(sql, rows, duration, lastError, values);
}
}
}
Expand All @@ -132,7 +131,7 @@ export abstract class Operator {
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
protected async _query(_sql: string): Promise<any> {
protected async _query(_sql: string, _values?: object | any[]): Promise<any> {
throw new Error('SubClass must impl this');
}

Expand Down
4 changes: 2 additions & 2 deletions src/transaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ export class RDSTransaction extends Operator {
}
}

protected async _query(sql: string) {
protected async _query(sql: string, values?: object | any[]) {
this.#check();
return await this.conn!._query(sql);
return await this.conn!._query(sql, values);
}

#check() {
Expand Down
6 changes: 3 additions & 3 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface RDSClientOptions extends PoolOptions {
}

export interface PoolConnectionPromisify extends Omit<PoolConnection, 'query'> {
query(sql: string): Promise<any>;
query(sql: string, values?: any | any[] | { [param: string]: any }): Promise<any>;
beginTransaction(): Promise<void>;
commit(): Promise<void>;
rollback(): Promise<void>;
Expand Down Expand Up @@ -63,8 +63,8 @@ export type LockTableOption = {
tableAlias: string;
};

export type BeforeQueryHandler = (sql: string) => string | undefined | void;
export type AfterQueryHandler = (sql: string, result: any, execDuration: number, err?: Error) => void;
export type BeforeQueryHandler = (sql: string, values?: object | any[]) => string | undefined | void;
export type AfterQueryHandler = (sql: string, result: any, execDuration: number, err?: Error, values?: object | any[]) => void;

export type TransactionContext = Record<PropertyKey, RDSTransaction | null>;
export type TransactionScope = (transaction: RDSTransaction) => Promise<any>;
Expand Down
16 changes: 8 additions & 8 deletions test/client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ describe('test/client.test.ts', () => {
[ table, prefix + '[email protected]' ]);
assert.deepEqual(mockLogs, [
'show tables',
`select * from \`${table}\` where email = '${prefix + '[email protected]'}' limit 1`,
'select * from ?? where email = ? limit 1',
]);
});
});
Expand Down Expand Up @@ -1478,7 +1478,7 @@ describe('test/client.test.ts', () => {
counter2After++;
});
await db.query('select * from ?? limit 10', [ table ]);
assert.equal(lastSql, 'select * from `myrds-test-user` limit 10');
assert.equal(lastSql, 'select * from ?? limit 10');
assert.equal(lastArgs[0], lastSql);
assert.equal(Array.isArray(lastArgs[1]), true);
assert.equal(count, 1);
Expand All @@ -1491,8 +1491,8 @@ describe('test/client.test.ts', () => {
values(?, ?, now(), now())`,
[ table, prefix + 'beginTransactionScope1', prefix + '[email protected]' ]);
});
assert.equal(lastSql, 'insert into `myrds-test-user`(name, email, gmt_create, gmt_modified)\n' +
` values('${prefix}beginTransactionScope1', '${prefix}[email protected]', now(), now())`);
assert.equal(lastSql, 'insert into ??(name, email, gmt_create, gmt_modified)\n' +
' values(?, ?, now(), now())');
assert.equal(lastArgs[0], lastSql);
assert.equal(lastArgs[1].affectedRows, 1);
assert.equal(count, 2);
Expand All @@ -1502,8 +1502,8 @@ describe('test/client.test.ts', () => {
values(?, ?, now(), now())`,
[ table, prefix + 'beginDoomedTransactionScope1', prefix + '[email protected]' ]);
});
assert.equal(lastSql, 'insert into `myrds-test-user`(name, email, gmt_create, gmt_modified)\n' +
` values('${prefix}beginDoomedTransactionScope1', '${prefix}[email protected]', now(), now())`);
assert.equal(lastSql, 'insert into ??(name, email, gmt_create, gmt_modified)\n' +
' values(?, ?, now(), now())');
assert.equal(lastArgs[0], lastSql);
assert.equal(lastArgs[1].affectedRows, 1);
assert.equal(count, 3);
Expand All @@ -1514,8 +1514,8 @@ describe('test/client.test.ts', () => {
values(?, ?, now(), now())`,
[ table, prefix + 'transaction1', prefix + '[email protected]' ]);
await conn.commit();
assert.equal(lastSql, 'insert into `myrds-test-user`(name, email, gmt_create, gmt_modified)\n' +
` values('${prefix}transaction1', '${prefix}[email protected]', now(), now())`);
assert.equal(lastSql, 'insert into ??(name, email, gmt_create, gmt_modified)\n' +
' values(?, ?, now(), now())');
assert.equal(lastArgs[0], lastSql);
assert.equal(lastArgs[1].affectedRows, 1);
assert.equal(count, 4);
Expand Down