Skip to content
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

Multiple Statement Execution (SERVER_MORE_RESULTS_EXISTS) #59

Closed
Kijewski opened this issue Mar 2, 2011 · 22 comments
Closed

Multiple Statement Execution (SERVER_MORE_RESULTS_EXISTS) #59

Kijewski opened this issue Mar 2, 2011 · 22 comments
Labels

Comments

@Kijewski
Copy link
Contributor

Kijewski commented Mar 2, 2011

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
Copy link
Collaborator

felixge commented Mar 2, 2011

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
Copy link
Contributor Author

Kijewski commented Mar 2, 2011

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
Copy link
Collaborator

felixge commented Mar 2, 2011

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

@Kijewski
Copy link
Contributor Author

Kijewski commented Mar 2, 2011

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

1 similar comment
@Kijewski
Copy link
Contributor Author

Kijewski commented Mar 2, 2011

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
Copy link
Contributor Author

Kijewski commented Mar 2, 2011

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
Copy link
Collaborator

felixge commented Mar 3, 2011

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
Copy link
Contributor Author

Kijewski commented Mar 4, 2011

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
Copy link
Collaborator

felixge commented Mar 4, 2011

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
Copy link
Contributor Author

Kijewski commented Mar 4, 2011

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

@tim-smart
Copy link

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

@Kijewski
Copy link
Contributor Author

Kijewski commented Mar 4, 2011

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

@aarong
Copy link

aarong commented Apr 6, 2011

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

@felixge
Copy link
Collaborator

felixge commented Apr 6, 2011

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

@jasonfutch
Copy link

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

@aarong
Copy link

aarong commented May 5, 2011

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

@Kijewski
Copy link
Contributor Author

Kijewski commented May 5, 2011

@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
Copy link

aarong commented May 6, 2011

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
Copy link

bminer commented Dec 7, 2011

+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
Copy link

bminer commented Dec 7, 2011

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

I like constructive criticism. :)

@bminer
Copy link

bminer commented Dec 29, 2011

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

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

@felixge
Copy link
Collaborator

felixge commented May 15, 2012

@felixge felixge closed this as completed May 15, 2012
dveeden pushed a commit to dveeden/mysql that referenced this issue Jan 31, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

6 participants