Fix call site when using Pool.prototype.query

[dougwilson]

This fixes the call site for long stack traces when using the query method on a pool object. Before the stack trace would only lead up to inside the getConnection method instead of the library caller. This results in the following change:

var mysql = require('mysql');
var pool = mysql.createPool(...);

pool.query('ERROR', function (err, rows) { // 1
  if (err) throw err;
  // The stack trace now includes the line marked 1
  // Prior to this, the stack trace included nothing from this file
});

[sidorares]

looks good! 👍

[ronkorving]

What's the value of this here? Should null or a self-like variable be passed instead?

[dougwilson]

All this did was move the previous

   conn.query(sql, values, function (err, rows, fields) {
     conn.release();
     cb.apply(this, arguments);
   });

The purpose is to cal the callback with the same this that conn.query would have called it with such that pool.query and conn.query have the same this instead of possible different ones. This allows people to use conn.query and pool.query interchangeably.

If you were wondering what this is pointing it, it is point to the connection.

[ronkorving]

I'm on a mobile device right now, so please forgive me for not diving deeper into this myself right now. It seems unlikely that "this" points to the connection from the createQuery callback though.

[sidorares]

It looks like it is actually a connection - 'this' for this anonymous function is set at https://github.com/felixge/node-mysql/blob/master/lib/protocol/sequences/Sequence.js#L78

[ronkorving]

Apologies, I stand corrected :)

[dougwilson]

@sidorares any news on merging this? :)

[sidorares]

oh, I was waiting for second 'OK'. @dresende - could you review please?

[felixge]

LGTM