-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
ON DUPLICATE KEY UPDATE produces wrong affectedRows #237
Conversation
Preparing to add the new pool feature.
Prepares PoolConfig / ServerConfig
For some reason the travis MySQL does not know the COM_CHANGE_USER command and gives a 'ER_UNKNOWN_COM_ERROR' error on both tests. See: http://travis-ci.org/#!/felixge/node-mysql/jobs/1849366
Node v0.4.x sometimes seems to emit a data event were the buffer is undefined. See: http://travis-ci.org/#!/felixge/node-mysql/jobs/1849436 This patch simplifies the parser code and handles this problem.
Mostly a cosmetic fix at this point.
Small things make life sweet.
Your test fails for me like this:
That's because MySQL actually returns:
Which is correct for this case. The MySQL command line does the same for me. (Did you also run the DROP TABLE when testing against the command line?) Please re-check your patch and let me know what you think. Re-open the ticket if you still think there is a problem in node-mysql. |
Here is the
I added better assert messages: https://github.com/ichernev/node-mysql/tree/on-duplicate-key-update affectedRows should be 0, because no row is inserted and no row is updated, but instead it is 1. |
re-opening this to investigate again |
In the first test ( |
@dresende So this is what the test does connection.query('INSERT INTO `' + table + '` (a, b, c) VALUES (1, 2, 3) ON DUPLICATE KEY UPDATE c = 1', function(err, info) {
assert.strictEqual(null, err);
assert.strictEqual(0, info.affectedRows, 'both primary key and updated key are the same so nothing is affected');
}); it checks that affectedRows is 0, and "the test should pass" -- all tests should pass, I don't know if it passes now or not. I want this test merged, so that the issue will be resolved (hopefully) in some future version. |
I typed |
|
This is probably related to |
There's a branch now where you can tweak flags without having to change the code. Check the commit on the branch |
This effectively shows that CLIENT_FOUND_ROWS is the responsible for having a different behavior from mysql command line client.
Merging the |
When you insert with on duplicate key update affected rows should be
The first case produces affected rows == 0 in the mysql console, and 1 in node_mysql.
I also wrote a test for it. I'm using