sidorares / sidorares/node-mysql2

Inconsistent ER_PARSE_ERROR

Open
#903 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
4.4k
Forks
680
Avg merge
9h 7m
Merged PRs (30d)
59

Description

Problem

Keep getting ER_PARSE_ERROR when running queries against a MySQL 5.6 database because the Node mysql driver fails to expand the value placeholders, so a query written like this (passing in [75] to the driver API)

const sql = `SELECT \`column_1\` FROM \`table_1\` WHERE \`id\` IN (?)`;

Is sent to the DB like this

SELECT `column_1` FROM `table_1` WHERE `id` IN (?)
{"time":"2018-12-11 21:26:33.877","level":"error","event":"data.source.db.sql","code":"ER_PARSE_ERROR","errno":1064,"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"}

What's even weirder is that if I deliberately introduce a mistake in the query, like a non-existent column in the SELECT, and then log out the query from MySQL mysql.general_log, the values are correctly expanded. Naturally, the query still fails but for a completely different reason!

SELECT `column_1`, `fakecolumn` FROM `table_1` WHERE `id` IN (75)

Could this be a genuine bug in the parser or am I not passing the array argument correctly to the query?

Code

Connections are retrieved from a pool configured like so

    const poolOpts = Object.assign({
        host,
        port,
        user,
        password,
        database,
        connectionLimit: CONNECTION_LIMIT,
        queueLimit: QUEUE_LIMIT,
        acquireTimeout: ACQUIRE_TIMEOUT,
        waitForConnections: WAIT_FOR_CONNECTIONS,
    }, connectionOptions);

    const pool = mysql.createPool(poolOpts);

This is the Node and the corresponding query. The query function we use is part of a mysql wrapper library (definition down below).

The value of the ids array which is passed in to the query method to expand the '?' placeholders is [ 23272531, 23272538], so I presume the value that gets injected in the end is [[23272531, 23272538]], which would expand the question mark as a comma-separated list of IDs without the parentheses, which I add myself in the SQL.

    async getValuesById(ids) {
        if (!ids || ids.length === 0) {
            throw new Error('No IDs were passed');
        }

        const sql = `
SELECT \`column_1\` FROM \`table_1\` WHERE \`id\` IN (?)
;`;

        try {
            const result = await this.dbSingle.query(
                sql,
                [ids],
                'data.source.db',
            );
            return result.map(row => row.column_1);
        } catch (e) {
            const error = `Failed to check if matches exist: ${e.toString()}`;
            this.logger.warn('data.source.db', { message: error, ids });
            throw new Error(error);
        }
    }

And here is the query() function definition in our mysql library wrapper.

    function releaseConnection(connection) {
        connection.release();
    }

    function newConnection() {
        return new Promise((resolve, reject) => {
            pool.getConnection((err, connection) => {
                if (err) {
                    logger.error('connector.DBConnection.newConnection', err);
                    return reject(err);
                }

                return resolve(connection);
            });
        });
    }

    function query(sql, values = [], label) {
        const outputLabel = label || DEFAULT_OUTPUT_LABEL;
        return new Promise((resolve, reject) => {
            const startToken = timers.start();
            newConnection()
                .then((connection) => {
                    connection.query(sql, values, (err, rows) => {
                        const duration = timers.stop(startToken);
                        if (err) {
                            logger.error(`${outputLabel}.sql`, { error: err.toString() });
                            connection.destroy();
                            return reject(err);
                        }
                        logger.info(`${outputLabel}.query.done`, {
                            duration,
                            count: rows.length,
                        });
                        releaseConnection(connection);
                        return resolve(rows);
                    });
                })
                .catch((err) => {
                    logger.error(`${outputLabel}.sql`, { error: err.toString() });
                    reject(err);
                });
        });
    }

How to Reproduce

I cannot reliably reproduce this issue in all of its different manifestations. There are some scenarios where I actually can, e.g. when I try to write to the database and get a duplicate row error back, and then carry on executing more unrelated queries (getting a new connection from the pool each time, as shown in newConnection()).

Environment

MySQL version: 5.6
Node mysql2 version: 1.6.4
Node version: 8.12.0
OS: Alpine Linux 3.6 (Docker)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the wrapper's query() function and its connection.query(sql, values, ...) call, then trace how the pool supplies connections. Reproduce the placeholder behavior using MySQL 5.6, Node 8.12.0, and mysql2 1.6.4; done means the failure is reliably isolated and the query either expands values consistently or the issue is shown to be outside the driver.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, mysql, nodejs
Domain
backend, database
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.