mysqljs / mysql

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

Multiple Statement Execution (SERVER_MORE_RESULTS_EXISTS) #59

Closed Kijewski closed 12 years ago

Kijewski commented 13 years ago

Hello Felix,

how difficult would it be to support multiple statements in one query, like BEGIN; SELECT 1; SELECT 2; COMMIT?

I think both having query(...) invoke the callback function multiple times and introduce something like multiQuery(...), where the callback gets arrays of rows and fields, would work fine.

Best regards, René

felixge commented 13 years ago

I think I want to support this. The main difficulty is that I don't know how many statements are contained in such a multiple statement query. Just counting the ;'s sounds like a rather bad approach.

Kijewski commented 13 years ago

Do you need to know the number of statements in prior? As long as there are more packets, the SERVER_MORE_RESULTS_EXISTS flag is set in serverStatus, isn't it?

felixge commented 13 years ago

I haven't come across that in the protocol documentation yet, do you know if it's documented somewhere?

Kijewski commented 13 years ago

MySQL's website does not seem to document the flag at all. The documentation of the EOF packet indicates, that SERVER_MORE_RESULTS_EXISTS is likely to occur at this place.

mysql_com.h defines the flag as follows:

define SERVER_MORE_RESULTS_EXISTS 8 /* Multi query - next query exists */

In my tests SERVER_MORE_RESULTS_EXISTS was set for all packets but those of the last “packet block” (i.e. the respective RESULT_SET_HEADER_PACKET to EOF_PACKET).

Kijewski commented 13 years ago

MySQL's website does not seem to document the flag at all. The documentation of the EOF packet indicates, that SERVER_MORE_RESULTS_EXISTS is likely to occur at this place.

mysql_com.h defines the flag as follows:

define SERVER_MORE_RESULTS_EXISTS 8 /* Multi query - next query exists */

In my tests SERVER_MORE_RESULTS_EXISTS was set for all packets but those of the last “packet block” (i.e. the respective RESULT_SET_HEADER_PACKET to EOF_PACKET).

Kijewski commented 13 years ago

Sorry, SERVER_STATUS_IN_TRANS should read SERVER_MORE_RESULTS_EXISTS. Somehow I copied the wrong word. :-/

I closed the issue by accident and now I can not re-open it. :-/

felixge commented 13 years ago

Awesome, that's exactly what I needed for implementing this : ). I don't expect to find time for this right away, but it's certainly coming.

Kijewski commented 13 years ago

I think https://github.com/Kijewski/node-mysql/commit/bbc107515311e146bfa0bf34b3d3e9898e035d27 should do the trick.

I did a few tests and it worked for me. BUT I did neither use TDD nor even wrote any unit tests, yet.

My commit would let Client.query call cb multiple time and introduces Client.multiQuery. I hope that API is fine with you. :)

felixge commented 13 years ago

Thanks. I'm quite the TDD nazi, so this won't make it into the driver without proper test coverage. So if you want to use it for now, I'd suggest you to use your own fork.

However, the work is much appreciated regardless, I will use it as the base when setting out to bring this into the driver and give you credit for it.

Kijewski commented 13 years ago

Thanks! Your code base was very well structured, so implementing this was quite easy. :)

tim-smart commented 13 years ago

Is this for START TRANSACTION; ... COMMIT; support? I am currently wanting this to do atomic inserts across multiple tables.

Kijewski commented 13 years ago

Yes, it is. For the same reason I needed this feature as well.

aarong commented 13 years ago

Any progress on this? Supporting transactions in this way would be a big step forward, IMO.

felixge commented 13 years ago

aarong: I haven't had a chance to adjust and test Kijewski's patch yet, but you can probably use it?

jasonfutch commented 13 years ago

Noticed that multiQuery is in the source... got a quick example on how to use it properly?

aarong commented 13 years ago

Has anybody gotten multiple statements to work correctly? I haven't had any success...

Kijewski commented 13 years ago

@aarong: My fork implements multiQuery. As I write my bachelor thesis atm, I won't find time to write a proper documentation or provide a proper patch for Felix. Have a look at the implementation to understand the callback value.

aarong commented 13 years ago

Works great and easy to figure out -- thank you. Hopefully this can be pulled into the main code base some time soon. Best of luck with the thesis.

bminer commented 12 years ago

+1 it would be super awesome to see this in the main code base. Named queues might be another way to implement multiple statements and transactions. For example, db.queueQuery('queue123', sql, input, cb) would queue that query on "queue123". Then, something as simple as db.execute('queue123') would kick it all off, ensuring no other queued queries execute until "queue123" is flushed. I dunno.

bminer commented 12 years ago

Check out this implementation... https://github.com/bminer/node-mysql-queues

I like constructive criticism. :)

bminer commented 12 years ago

I have added transaction support to node-mysql. Check out this project:

https://github.com/bminer/node-mysql-queues

felixge commented 12 years ago

Implemented in 2.0.0-alpha, see: http://debuggable.com/posts/releasing-node-mysql-2-0-0-alpha:4fb21c18-7f84-4e8c-a1ef-599bcbdd56cb