pool cluster node pattern select one error

[tdlrobin]

_serviceableNodeIds is Array not hash object, when pattern is a nodeId you can not use it to index

[felixge]

Can you make a test that shows the issue?

[tdlrobin]

var mysql = require('mysql');

var poolCluster = mysql.createPoolCluster();

poolCluster.add("SHARD01", {
host: "127.0.0.1",
user: "root",
password: "",
database: "test1",
connectionLimit: 1
});

poolCluster.add("SHARD02", {
host: "127.0.0.1",
user: "root",
password: "",
database: "test2",
connectionLimit: 1
});

/***
at test2 database:

CREATE TABLE ttt (
id int(11) DEFAULT NULL
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

*/

poolCluster.getConnection("SHARD02", function (err, connection) {
if(err){
console.error(err);
return;
}
connection.query("select * from ttt", function (err, result) {
if(err){
console.log(err);
return;
}
connection.release();
console.log(result);
});
});

this will get err:

{ [Error: ER_NO_SUCH_TABLE: Table 'test1.ttt' doesn't exist]
code: 'ER_NO_SUCH_TABLE',
errno: 1146,
sqlState: '42S02',
index: 0 }

##########

this code cannot match

} else if (typeof this._serviceableNodeIds[pattern] !== 'undefined') { // one
foundNodeIds = [pattern];
}

it match the else:

else { // wild matching
var keyword = pattern.substring(pattern.length - 1, 0);

foundNodeIds = this._serviceableNodeIds.filter(function (id) {
  return id.indexOf(keyword) === 0;
});
}

but keyword will be "SHARD0",will match SHARD01, not match SHARD02.

[tdlrobin]

I change the test-cluster.js code to test this bug:

var common = require('../../common');
var assert = require('assert');

function createPoolCluster(clusterConfig, poolConfig) {
    var cluster = common.createPoolCluster(clusterConfig);

    if (typeof poolConfig === 'undefined') {
        poolConfig = common.getTestConfig();
    }

    cluster.add(poolConfig);
    cluster.add('MASTER', poolConfig);
    cluster.add('SLAVE01', poolConfig);
    cluster.add('SLAVE02', poolConfig);

    return cluster;
}

// Test_getConnection_one
(function() {
    var cluster = createPoolCluster();

    cluster.getConnection('SLAVE02', function(err, connection) {
        cluster.end();

        if (!err) {
            assert.strictEqual(connection._clusterId, 'SLAVE02');
        }
    }.bind(this));
})();

// Test_of_getConnection_one
(function() {
    var cluster = createPoolCluster();

    cluster.of('SLAVE02').getConnection(function(err, connection) {
        cluster.end();

        if (!err) {
            assert.strictEqual(connection._clusterId, 'SLAVE02');
        }
    }.bind(this));
})();

[tdlrobin]

This is a very serious and obvious problem, why nobody cares?

_serviceableNodeIds is init by array, not the hash object ( this._serviceableNodeIds = {}; )

this._serviceableNodeIds = [];
...
...

this._serviceableNodeIds.push(id);

but in fact wrong use it as hash object :

if (typeof this._serviceableNodeIds[pattern] !== 'undefined')

pattern is int ? but in fact pattern it used to be nodeId or pattern (' * ') ,you expect use it to get any thing from _serviceableNodeIds?

[felixge]

Merging this. It looks correct, but I have no time for more detailed review. If anybody objects, please speak up.