mysqljs / mysql

A pure node.js JavaScript Client implementing the MySQL protocol.
MIT License
18.26k stars 2.53k forks source link

Stored Procedure with an Insert statement not returning the expected result #1818

Open marc324 opened 6 years ago

marc324 commented 6 years ago

I am working on a project that involves Node and a MySQL database. I have a stored procedure that looks like this:

CREATE PROCEDURE insertUser(
    IN $oID INT(11), 
    IN $uName VARCHAR(40),
    IN $uSurname VARCHAR(40),
    IN $uEmail VARCHAR(255))
BEGIN
    DECLARE EXIT HANDLER FOR SQLEXCEPTION
    BEGIN 
        ROLLBACK; 
    END;
    START TRANSACTION;
    IF NOT EXISTS(Select 1 from Organisations where oID = $oID) THEN 
        Select -1 as error;
    ELSE
        IF EXISTS(Select 1 from Users where uEmail = $uEmail) THEN
            Select -2 as error;
        ELSE
            Insert into Users 
                    (uOrganisationID, uName, uSurname, uEmail)
            Values  ($oID, $uName, $uSurname, $uEmail);

            Select uID, uOrganisationID, uName, uSurname, uEmail
            from Users
            where uEmail = $uEmail;
        END IF; 
    END IF;
    COMMIT;
END

The JS code looks like this:

const q = `
CALL insertUser(${escape(data.uOrganisationID)}, 
            ${escape(data.uName)},
            ${escape(data.uSurname)},
            ${escape(data.uEmail)});`;
executeQuery(q)
      .then((results) => {
          ...some logic...
      })
      .catch((err) => {
          ...some logic...
      });

executeQuery just gets a new connection from the pool and executes the query all wrapped in a promise.

The stored procedure works fine in MySQL Workbench but it does not always work when executed from JS.

I get 3 different results: I get the following when uOrganisationID does not exist. This is the result that I expect so Correct.

[ [ RowDataPacket { error: -1 } ], OkPacket {...} ]

I get the following when uEmail already exist. This is also the result that I expect so Correct again.

[ [ RowDataPacket { error: -2 } ], OkPacket {...} ]

I get the following when validations pass. This is NOT the correct result in my view as I expect to see the results of the Select statement above.

OkPacket {...}

Is this the expected result? As mentioned, I expect to get an array with the result of the Select statement above in the first position.

Many thanks in advance for any help.

sidorares commented 6 years ago

which select result you expect to see in third case? In case only this block is executed

            Insert into Users 
                    (uOrganisationID, uName, uSurname, uEmail)
            Values  ($oID, $uName, $uSurname, $uEmail);

            Select uID, uOrganisationID, uName, uSurname, uEmail
            from Users
            where uEmail = $uEmail;

you'll only get result from insert ( which is returned as OkPacket )

dougwilson commented 6 years ago

I mean, I think there should be a second resultset for that select, as it's a separate statement in the procedure, right? @marc324 can you add debug: true to your connection/pool options, run that third condition on your MySQL server and paste here the debug output printed on STDOUT?

sidorares commented 6 years ago

oops, you are right @dougwilson in that block they are both insert and select, I'd expect both of them to be in response. Might actually be a bug

marc324 commented 6 years ago

Thank you very much for your help.

@dougwilson, I have added debug: true into my connection settings and I believe that this is the output that you were interested in.

--> ComPingPacket
ComPingPacket { command: 14 }

<-- OkPacket
OkPacket {
  fieldCount: 0,
  affectedRows: 0,
  insertId: 0,
  serverStatus: 2,
  warningCount: 0,
  message: '',
  protocol41: true,
  changedRows: 0 }

--> ComQueryPacket
ComQueryPacket {
  command: 3,
  sql: '\nCALL insertUser(48,\n              \'A user\',\n              \'A user surname\',\n              \'user@example.com\',\n              NULL);\n' }

<-- OkPacket
OkPacket {
  fieldCount: 0,
  affectedRows: 0,
  insertId: 0,
  serverStatus: 2,
  warningCount: 2,
  message: '',
  protocol41: true,
  changedRows: 0 }

I get this output when I execute a bunch of unit tests.

As I mentioned in the first comment, the expected output for the third case is an array with the results of the select statement in the first position of it.

Again, thank you for your help.

dougwilson commented 6 years ago

Very weird. That OK packet is likely the OK result of the first INSERT, but the server status does not have the more results flag set, so we assume that it's the last result set from the query. I may need to play around with this locally. Sometimes there can be odd behavior wit specific MySQL server versions. Can you let me know what server software and version you are using?

marc324 commented 6 years ago

The database is hosted in a test server in a raspberry pi 3 running Centos 7. Output of mysql --version command: mysql Ver 15.1 Distrib 5.5.52-MariaDB, for Linux (armv7l) using readline 5.1

Please let me know if I can be of any help to resolve this.

marc324 commented 6 years ago

@dougwilson, thank you for the help last week. I just wanted to know if you had a chance to investigate this issue? Thank you.

dougwilson commented 6 years ago

Not yet.

kai-koch commented 6 years ago

When you run the unit test, please make sure each test uses an unique email and organisation.

Theoretically, depending on your test setup and if the fields are unique, if you fire your test all in the same process tick, they could interfere with each other, if they use the same email/organisation.

e.g. if the test-conditons (SELECT -1 ...; and SELECT -2 ...;) are clear, but another test on a different pool-connection had insert the same Organisation/Email between the "SELECT"-Tests and the INSERT-Statement, an error would occur and the error handler would rollback the insert, which would give you no result set from the select, because the other test might not have been committed yet.

That the procedure works in workbench, but not always in the unit tests, indicates some kind of race condition, IMO. Just like the one I tried to construct above.