-
-
Notifications
You must be signed in to change notification settings - Fork 622
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
Problems with promise based pools after updating to 3.6.0 #2192
Comments
Hi @BauerPh could you add some code to your example that can replicate the issue on a fresh database so I can debug it locally? Thanks |
@sidorares I'm running into the same issue when using import mysql from 'mysql2/promise'
import sql from 'sql-template-tag'
const db = mysql.createPool(...)
await db.query(sql`SELECT 1`) If I do the following, 3.6.2 works though: import mysql from 'mysql2/promise'
import sql from 'sql-template-tag'
const db = mysql.createPool(...)
const query = sql`SELECT 1`
await db.query({ sql: query.sql, values: query.values }) |
@jgonera looks like a side effect of https://github.com/sidorares/node-mysql2/pull/2159/files#diff-f35455f30798a56117f979b6ad64a99cbc915318f08795935d68c80ab2223743R623-R626 sql-template-tag uses getters to return different 'view' of the query for mysql and pg - https://github.com/blakeembrey/sql-template-tag/blob/ee664d608d8ca82c2eb01302e9efd00b1b02b6fa/src/index.ts#L76-L81 simpler example demonstrating the issue: import sql from 'sql-template-tag'
const query = sql`SELECT ${"test"}`
console.log('via getters: ', query.sql, query.values)
const options = { ...sql }
console.log('after spread: ', options.sql, options.values) outputs
Possible solution would be to change the code in connection.js/createQuery from options = {
...options,
...sql
}; to options = {
...options,
sql: sql.sql,
values: sql.values
}; Do you want to test that and create PR @jgonera ? |
your solution worked for me. PR created, please review. I only changed it in connection.js/createQuery, |
yes, probably needs to be added to execute as well |
fixed v3.6.2 |
@BauerPh thank you for the fix and sorry for not getting to it earlier! |
Hi, the issue still seems to exist in 3.6.2 when used with https://github.com/nearform/sql . The fix in #2238 explicitly sets the sql option but not the values option. const run = async () => {
const SQL = require('@nearform/sql');
const { createConnection } = require('mysql2/promise')
const con = await createConnection(config);
const id = 1;
const sql = SQL`
SELECT * FROM mytable WHERE ID = ${id}
`;
await con.query(sql)
await con.close()
};
run();
/*
Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '?' at line 1
at PromiseConnection.query (/node_modules/mysql2/promise.js:94:22)
at run (/repro.js:15:15)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5) {
code: 'ER_PARSE_ERROR',
errno: 1064,
sql: 'SELECT * FROM mytable WHERE ID = ?',
sqlState: '42000',
sqlMessage: "You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '?' at line 1"
}
*/ |
@fiacc could you submit a PR with |
I'm still open to a first class tagged template literal support out of the box, see #1539 ( but also #1539 (comment) ) |
I get this Error after updating to 3.6.0:
it works fine with the previous version 3.5.2
Here is my call:
The text was updated successfully, but these errors were encountered: