ON DUPLICATE KEY UPDATE produces wrong affectedRows

[ichernev]

When you insert with on duplicate key update affected rows should be

• 0 when both the key is the same, and the update statement doesn't change anything
• 1 when the key differs, so a row is inserted
• 2 when the key is the same, and the row is updated

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

% mysql --version
mysql  Ver 14.14 Distrib 5.5.24, for Linux (x86_64) using readline 5.1

[felixge]

Your test fails for me like this:

AssertionError: both primary key and updated key are the same so nothing is affected
    at Query._callback (/Users/felix/code/node-mysql/test/integration/test-on-duplicate-key-update.js:22:10)
    at Query.end (/Users/felix/code/node-mysql/lib/protocol/sequences/Sequence.js:66:24)
    at Query._handleFinalResultPacket (/Users/felix/code/node-mysql/lib/protocol/sequences/Query.js:138:8)
    at Query.OkPacket (/Users/felix/code/node-mysql/lib/protocol/sequences/Query.js:72:10)
    at Protocol._parsePacket (/Users/felix/code/node-mysql/lib/protocol/Protocol.js:156:23)
    at Parser._onPacket (native)
    at Parser.write (/Users/felix/code/node-mysql/lib/protocol/Parser.js:60:12)
    at Protocol.write (/Users/felix/code/node-mysql/lib/protocol/Protocol.js:31:16)
    at Socket.ondata (stream.js:36:26)
    at Socket.emit (events.js:64:17)

That's because MySQL actually returns:

{ fieldCount: 0,
  affectedRows: 2,
  insertId: 0,
  serverStatus: 2,
  warningCount: 0,
  message: '' }

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.

[ichernev]

Here is the mysql console output:

mysql> delete from on_duplicate_key_test;
Query OK, 2 rows affected (0.00 sec)

mysql> insert into on_duplicate_key_test values (1,1,1);
Query OK, 1 row affected (0.00 sec)

mysql> insert into on_duplicate_key_test values (1,2,3) on duplicate key update c = 1;
Query OK, 0 rows affected (0.00 sec)

mysql> insert into on_duplicate_key_test values (2,3,4) on duplicate key update c = 1;
Query OK, 1 row affected (0.00 sec)

mysql> insert into on_duplicate_key_test values (1,2,3) on duplicate key update c = 2;
Query OK, 2 rows affected (0.00 sec)

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. mysql client is consistent on this.

[felixge]

re-opening this to investigate again

[dresende]

In the first test (both primary key and updated key are the same so nothing is affected), using info.changedRows should be zero and the test should pass.

[ichernev]

@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.

[dresende]

I typed changedRows, not affectedRows :)

[ichernev]

@dresende, ah sorry. But then the cli reports "rows affected" which makes sense to be connected to info.affectedRows. If the number reported in the cli as rows affected is actually stored in info as changedRows, then I guess the tests should be changed and merged.

@felixge what do you think?

[dresende]

affectedRows is not the same as "rows affected" in mysql cli. affectedRows comes directly from the mysql protocol. changedRows comes from parsing the OK message (if any). If you see a different value that's what mysql returns. Probably mysql cli display OK message when it exists, otherwise it parses the OK packet.

[dresende]

This is probably related to CLIENT_FOUND_ROWS flag in the ConnectionConfig.js. We could eventually change this or have the possibility to switch on/off somes flags.

[dresende]

There's a branch now where you can tweak flags without having to change the code. Check the commit on the branch connection_flags.

dc04c61

[dresende]

Merging the connection_flags branch will allow people to control the connection flags and have the affectedRows as they would like.