Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

Support stored procedure. #26

Closed
wants to merge 5 commits into from
Closed

Support stored procedure. #26

wants to merge 5 commits into from

Conversation

yihuang
Copy link

@yihuang yihuang commented Nov 12, 2012

It don't break exist tests, and pass the test i add for stored procedure.
The first commit is a hexdump utility i use to solve this problem, if you don't like it, you can just pick the rest commits (the second commit is the changes need to support stored procedure, the third one is the test).

@jskorpan
Copy link

Thanks for the contribution!

Are there any other UltraMySQL users that could take part in validating and expanding on this patch?

Sadly I have no experience what so ever with Stored Procedures.

//JT

@mthurlin
Copy link
Member

@yihuang Your patch only sets the MULTI_RESULT protocol flag, does ultramysql really support multiple result sets?
Your tests does not test this, so I would like a test with the following stored procedure:

DELIMITER //
CREATE PROCEDURE TestMultiResult()
BEGIN
    SELECT 1;
    SELECT 2;
    SELECT 3;
END //
DELIMITER ;

Will it return three result sets correctly?

@yihuang
Copy link
Author

yihuang commented Nov 14, 2012

Yes, it don't really support MULTI_RESULT, but i can try to implement this feature in following days.
I don't have much experiences at mysql protocol, any suggestion would be appreciated.

@jskorpan
Copy link

Sounds awesome, keep us posted!

@yihuang
Copy link
Author

yihuang commented Nov 14, 2012

It's done.
It needs server support CLIENT_PROTOCOL_41, the code don't check that, because existing code seems assuming that already.

@jskorpan
Copy link

Hi, sorry for the slow response.
I've merged your pull request into a separate branch. Such a big feature addition should be taken slowly and must be thorowly tested.

Thanks for your awesome contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants