sidorares / node-mysql2

:zap: fast mysqljs/mysql compatible mysql driver for node.js
https://sidorares.github.io/node-mysql2/
MIT License
4.04k stars 610 forks source link

affectedRows value is incorrect. no changedRows value in result #875

Open ianbale opened 5 years ago

ianbale commented 5 years ago

Hi,

I've looked through all the old issues and can see that changedRows value ought to be present in the result set now. However I am not seeing it...

I've also noticed that affectedRows has the wrong value when doing an insert

This is my query

connection.DM.query('insert into trainServices (serviceId,depart,origin,destination,coaches) 
values (?,?,?,?,?) on duplicate key update coaches = values(coaches)',
[service.serviceId,service.depart,service.origin,service.destination,JSON.stringify(coaches)])

And this is the result data when it results in an insert:

{
    "fieldCount": 0,
    "affectedRows": 2,
    "insertId": 0,
    "info": "",
    "serverStatus": 2,
    "warningStatus": 0
  },

Here I expected affectedRows to have a value of 1 since only 1 row was inserted.

affectedRows has the expected value of 1 when the query does not contain the on duplicate key part.

This is the result when there is no change to any data Or when there is a duplicate so it does an update and there is a change to the data

  {
    "fieldCount": 0,
    "affectedRows": 1,
    "insertId": 0,
    "info": "",
    "serverStatus": 2,
    "warningStatus": 0
  },

Here I expected to see changeRows with a value of 0 when there was no data change and a value of 1 when there was, but it is not present in the result set. I also notice that info is empty.

I am using mysql2 v1.6.3 (promise mode) and my DB is an AWS RDS instance running mySQL v 5.6.41

Has changedRows support been dropped? Or is there any known issue with AWS's mySQL implementation that is preventing mysql2 picking up the necessary query response info?

Thanks for your time

ianbale commented 5 years ago

I've just tried splitting my query into separate insert and updates I'm now seeing the changedRows value.

So it looks like support for that was missed from the on duplicate key update method...?

sidorares commented 5 years ago

Hi @ianbale

changedRows is extracted from text message. It's not part of main protocol and less stable ( if message format changes because of locale or mysql server version update it might fail ). The code is here - https://github.com/sidorares/node-mysql2/blob/c955041b0785bb7b7cc5536982fab0a418e70a88/lib/packets/resultset_header.js#L86

affectedRows on the other hand is part of the header packet - https://github.com/sidorares/node-mysql2/blob/c955041b0785bb7b7cc5536982fab0a418e70a88/lib/packets/resultset_header.js#L28

sidorares commented 5 years ago

Can you post log around that query with debug turned on?

ianbale commented 5 years ago

So that incorrect value for affectedRows - 2 when only 1 row was inserted is coming from mySQL?

I've added the debug option to my connection config, but I am not getting anything useful out. This code is in an AWS Lambda function. I'm running it locally using serverless-offline, but all I am getting in the way of debug output for the query is:

request.js:247 Debug: internal, implementation, error Error: Uncaught error: Unexpected packet while no commands in the queue at Connection.protocolError (/Users/ianbale/Source Code/aws-reservations/node_modules/mysql2/lib/connection.js:388:17) at Connection.handlePacket (/Users/ianbale/Source Code/aws-reservations/node_modules/mysql2/lib/connection.js:448:12) at PacketParser.onPacket (/Users/ianbale/Source Code/aws-reservations/node_modules/mysql2/lib/connection.js:73:18) at PacketParser.executeStart (/Users/ianbale/Source Code/aws-reservations/node_modules/mysql2/lib/packet_parser.js:75:16) at Socket. (/Users/ianbale/Source Code/aws-reservations/node_modules/mysql2/lib/connection.js:80:31) at emitOne (events.js:116:13) at Socket.emit (events.js:211:7) at addChunk (_stream_readable.js:263:12) at readableAddChunk (_stream_readable.js:250:11) at Socket.Readable.push (_stream_readable.js:208:10) at TCP.onread (net.js:607:20)

jimmyolo commented 5 years ago

try connection option: flags: "-FOUND_ROWS", ?

ianbale commented 5 years ago

Sorry for the delay getting back about this. I've had to focus on some other work and have not yet had another opportunity to look at this. I've left this window open in my browser so I notice it every day. I will get back to it once the current pressing job is complete.

PhilSwift7 commented 5 years ago

from mysql documentation: https://dev.mysql.com/doc/refman/8.0/en/mysql-affected-rows.html For INSERT ... ON DUPLICATE KEY UPDATE statements, the affected-rows value per row is 1 if the row is inserted as a new row, 2 if an existing row is updated, and 0 if an existing row is set to its current values. If you specify the CLIENT_FOUND_ROWS flag, the affected-rows value is 1 (not 0) if an existing row is set to its current values.

aikar commented 5 years ago

Thank you @victor0801x you saved us....

@sidorares why is FOUND_ROWS a default flag? that really screwed us up as it breaks the ability to know if a row was modified or not, and creates different results than running queries on workbench/command line

That should not be a default on flag....

MatthewJohnSymons commented 5 years ago

I'd also like to thank @victor0801x. After spending hours trying to figure out why my application was behaving differently using node-mysql2 vs running the stored procedures in workbench, the above solution of removing the FOUND_ROWS flag has solved it.

I agree with @aikar, this should not be a default flag. At the very least it should be documented in regards to what the defaults are so people are aware. This has been generating unexpected results.

jimmyolo commented 5 years ago

@aikar @MatthewJohnSymons I think it's default behavior in mysql. if you using mysql-client cli connector and typing those command will get the same results as mysql2

mysql -p
use test;
CREATE TABLE t(id INT, name VARCHAR(48), PRIMARY KEY(id));
INSERT INTO t VALUES (1, 'jimmy'),(2, 'huang');
-- Query OK, 2 rows affected (0.002 sec)
-- Records: 2  Duplicates: 0  Warnings: 0
-- +----+-------+
-- | id | name  |
-- +----+-------+
-- |  1 | jimmy |
-- |  2 | huang |
-- +----+-------+

INSERT INTO t VALUES (1, 'jimmy') ON DUPLICATE KEY UPDATE name='jimmy';
-- Query OK, 0 row affected (0.002 sec)

INSERT INTO t VALUES (1, 'jimmy') ON DUPLICATE KEY UPDATE name='huang';
-- Query OK, 2 rows affected (0.001 sec)

INSERT INTO t VALUES (3, 'jimmy') ON DUPLICATE KEY UPDATE name='jimmy';
-- Query OK, 1 rows affected (0.001 sec)

UPDATE t SET name='huang' WHERE id=2;
-- Query OK, 0 rows affected (0.002 sec)
-- Rows matched: 1  Changed: 0  Warnings: 0

UPDATE t SET name='jimmy' WHERE id=2;
-- Query OK, 1 row affected (0.001 sec)
-- Rows matched: 1  Changed: 1  Warnings: 0
trevorr commented 5 years ago

This just wasted a few hours of my life too. FOUND_ROWS is probably enabled by default because it is in mysqljs/mysql. Of course, their one documented example of disabling a default flag is for that one, so I'll take that as some acknowledgement that maybe it shouldn't have been made a default.

aikar commented 5 years ago

@victor0801x the results you pasted show the opposite, the first insert says 0 rows found, when FOUND_ROWS should of made it say 1.