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

Multi Result support #66

Closed
emilsjolander opened this issue Apr 29, 2013 · 33 comments
Closed

Multi Result support #66

emilsjolander opened this issue Apr 29, 2013 · 33 comments

Comments

@emilsjolander
Copy link

@emilsjolander emilsjolander commented Apr 29, 2013

I am getting the following error when calling on my stored procedure

Error 1312: PROCEDURE mydb.GetAllNotes can't return a result set in the given context

I am calling on my stored procedure with this code

db.Query("CALL MyStoredProcedure()")
@arnehormann
Copy link
Member

@arnehormann arnehormann commented Apr 30, 2013

Hi @emilsjolander, thanks for reporting this!
I don't know about Julien, but I haven't used stored procedures yet.
Can you provide SQL and go code for a failing case (play.golang.org or gist)?
It would be faster than us (or at least me) rewriting that code.

@emilsjolander
Copy link
Author

@emilsjolander emilsjolander commented Apr 30, 2013

I actually got them working by following the instructions in the last comment of this thread https://groups.google.com/forum/?fromgroups=#!topic/golang-nuts/raVDmAzkZ5U

adding

clientMultiStatements |
   clientMultiResults |

to line 209 in packets.go

@arnehormann
Copy link
Member

@arnehormann arnehormann commented Apr 30, 2013

@julienschmidt I can't really say what this does to the driver - e.g. when you really have multiple results per query, how do you get the next one with the current api? Can you determine if there's only one result returned by a stored procedure? My guess would be we can't include this, but I just don't know. Maybe yet another dsn switch for those who really need it?

@emilsjolander
Copy link
Author

@emilsjolander emilsjolander commented Apr 30, 2013

well i kinda knew it wasn't a fix, but i needed to quickly get a project for a school working ^^

@arnehormann
Copy link
Member

@arnehormann arnehormann commented Apr 30, 2013

Still, thanks @emilsjolander! Can you provide some code? Maybe we'll add it to the tests, we'll have to check/discuss this.

@julienschmidt
Copy link
Member

@julienschmidt julienschmidt commented Apr 30, 2013

I know about this issue. Does it work with clientMultiStatements flag only for you?

There is no way to determine how many results the procedure produces.
With the current database/sql API it is impossible to handle multiple results (at least in a way that makes sense).
I'm going to make a design proposal for that after the Go1.1 release.

The clientMultiStatements is not present because I don't know a case where multiple statements don't also produce multiple results. This is also a nice way to prevent SQL injections.
But it might be the case, that it works with stored procedures. So give it a try ;)

@arnehormann
Copy link
Member

@arnehormann arnehormann commented May 14, 2013

Hi @emilsjolander, can you please test Juliens suggestion and get back to us?

@sdming
Copy link

@sdming sdming commented Jul 25, 2013

Did this issue fixed?

I got same error when call procedure with code

CALL sp_a(?,?,?) 

The third parameter is output parameter, so I change code to following

SET @sp_var_p1 = ?;
SET @sp_var_p2 = ?;
CALL sp_a ( @sp_var_p1, @sp_var_p2, @sp_var_p3 );
SELECT @sp_var_p3;

I got another error "check the manual that corresponds to your MySQL server version for the right syntax to use near 'SET @sp_var_p2 = ?; "

@arnehormann
Copy link
Member

@arnehormann arnehormann commented Jul 25, 2013

Go does not support multiple results in database/sql (yet). The driver closely matches the api, that's why it's not supported here. Support for multiple statements per query is missing as well. They could produce multiple results which the driver could not handle in a sensible way. That's a limitation for stored procedures, too.
We didn't follow up on this issue because we don't have test code for stored procedures and adding both clientMultiStatements and clientMultiResults will not be supported by the driver until Go is multi result capable in the core library. For your use case, clientMultiStatements is probably sufficient.
Please clone the driver, change packets.go to include clientMultiStatements in the flags and tell us if it works for you that way.
If it does, we'll discuss adding it to the driver in some way (e.g. configurable with a dsn-flag).

@sdming
Copy link

@sdming sdming commented Jul 26, 2013

changed code to

    clientFlags := uint32(  
        clientProtocol41 |  
            clientSecureConn |  
            clientLongPassword |  
            clientTransactions |  
            clientMultiStatements |  
            clientMultiResults | // https://github.com/go-sql-driver/mysql/issues/66  
            clientLocalFiles,  
    ) 

1: it works

    CALL sp_query ( ?, ?, ? );
CREATE PROCEDURE `sp_query`(IN p1 INT, IN p2 varchar(100), p3 NUMERIC(10,4))
BEGIN    
   SELECT p1,  p2, p3;
END

2: it works

    CALL sp_exec ( ?, ?, ? );
CREATE PROCEDURE `sp_exec`(IN p1 INT, IN p2 varchar(100), p3 NUMERIC(10,4))
BEGIN    
   set p1 = 123;
   set p2 = "p2";
   set p3 = 3.14;
END

3: it dones't work

SET @p2 = ?; 
CALL sp_exec_inout ( ?, @p2, @p3 );
SELECT @p2, @p3; 
CREATE PROCEDURE `sp_exec_inout`(IN p1 INT, INOUT p2 varchar(100), OUT p3 NUMERIC(10,4))
BEGIN    
   set p1 = 123;
   set p2 = "p2";
   set p3 = 3.14;
END
@arnehormann
Copy link
Member

@arnehormann arnehormann commented Jul 26, 2013

Does it need clientMultiResults or does it work without it (using only clientMultiStatements)?

@ghost ghost assigned julienschmidt Nov 29, 2013
@julienschmidt julienschmidt removed this from the v1.2 milestone Apr 1, 2014
@mattes
Copy link

@mattes mattes commented Sep 16, 2014

Any updates on this? I just used clientMultiStatements in the clientFlags and it works fine.

I use this in https://github.com/mattes/migrate where usually just one SQL file with n sql commands is being executed.

@mattes
Copy link

@mattes mattes commented Sep 16, 2014

It's actually not working ok. Multiple DROP TABLE worked, but multiple CREATE TABLE commands failed:

Commands out of sync. Did you run multiple statements at once?

@arjitkgupta
Copy link

@arjitkgupta arjitkgupta commented Oct 9, 2014

I have asked about returning result from stored procedure in #282 . But as @julienschmidt had answerd that adding
clientMultiStatements |
clientMultiResults |
is not the solution of the issue.
And in http://go-database-sql.org/surprises.html which had refered this page only that calling SP from mysql is not possible right now.

As i want to call sp from go-sql-driver i want to ask how it is possible ?

@arnehormann
Copy link
Member

@arnehormann arnehormann commented Oct 9, 2014

The driver does not support stored procedures because the Go api in database/sql does not provide the necessary parts (something like sql.Rows having a NextRows() in addition to Next()).
We can't bypass database/sql, so we can't provide stored procedure support as it could lead to multiple results from a single query.
So that's it. It's impossible unless the database/sql api changes.

@nomad-software
Copy link

@nomad-software nomad-software commented Feb 15, 2015

Is there any movement on this issue as i really need to execute multiple statements. I'm not really fused about return multiple results as i'm just creating databases from SQL dumps.

If this is not being implemented why not expose the mysqlConn.flags in the public API so at least we can set those on a case by case basis without needing to fork this entire repo?

@mtjburton
Copy link

@mtjburton mtjburton commented Feb 28, 2015

@nomad-software

I'm in a similar boat to you right now I'm reading the SQL from files on disk for the purposes of DB migrations and don't want to unnecessarily have separate files for each CREATE TABLE for example. So currently I'm working round the issue by ensuring each of my statements ends with a semi-colon and splitting on this in code using something similar to below.

statement := "CREATE TABLE vendor(id INT); CREATE TABLE product(id INT);"

for _, s := range strings.Split(statement, ";") {
    trimmed := strings.TrimSpace(s)
    if len(trimmed) > 0 {
        _, err := db.Exec(trimmed)
        _ = err
    }
}
@nomad-software
Copy link

@nomad-software nomad-software commented Feb 28, 2015

@mtjburton

That is one solution if you have control over the SQL dumps. In my case the dumps contain SQL like this:

DELIMITER $$

CREATE DEFINER=`gary`@`localhost` FUNCTION `hello_world`() RETURNS text CHARSET utf8 COLLATE utf8_bin
BEGIN
    RETURN 'Hello World';
END$$

DELIMITER ; 

So splitting on semi-colons with result in errors. In the end i used this library: https://github.com/ziutek/mymysql which supports multi-statements while not adhering to the stdlib interface.

@llooz
Copy link
Contributor

@llooz llooz commented Apr 12, 2015

I have just tried to use stored procedures statements (adding clientMultiStatements|clientMultiResults) but i ended up with strange busy buffer errors. If using stored procedure with just a single result without output parameters there is no difference for the database/sql interface and i think that cases like this should be supported.
For now i switched to mymysql using a godrv (database/sql) that discards all other result sets after the first one

@llooz
Copy link
Contributor

@llooz llooz commented Apr 12, 2015

I think that i have reached a good point.
To be able to use stored procedures with only one result i have enabled clientMultiResults and discarded all other results by checking the status flag of EOF packets statusMoreResultsExists
See llooz@9542bdd

@hansiemithun
Copy link

@hansiemithun hansiemithun commented Apr 17, 2015

@Idhor, facing the same problem still after cloning from your repo. Could you please brief a bit about with an small example.

@llooz
Copy link
Contributor

@llooz llooz commented Apr 19, 2015

@hansiemithun
I'm not using it in production yet but i have tried right now and it works

var id int
country := "Italy"
row := db.QueryRow("CALL GetCountryId(?)", country)
if err := row.Scan(&id); err != nil {
    return 0, err
}
@PuppyKhan
Copy link

@PuppyKhan PuppyKhan commented Apr 22, 2015

I have been able to consistently reproduce issue #314 by attempting to used the clientMultiStatements | clientMultiResults hack with the following test util: https://github.com/PuppyKhan/dbhammer

Use the following params to see the error in action: dbhammer -db=mysql -conns=1 -sp

You need to preset a couple of stored procedures (see docs) and basically limit the # of connections to force them to be reused by the next query and the issue appears.

I strongly suggest running this or a similar test before considering any such hack production ready, though @Idhor 's work looks the most promising.

@badoet
Copy link
Contributor

@badoet badoet commented May 11, 2015

I think it would be good to at least enable the clientMultiStatements or clientMultiResults through the dsn params. this way, it will be an optional flag just like the interpolate params.
editing the packets.go directly can be overwritten when running go get -u
forking this library just to change the packets.go is super inefficient and create more maintenance work.
the clientMultiStatements is very useful for batch update. the multi results issue is not important in this instance.

UPDATE:
submitted pull request #339

llooz referenced this issue in llooz/mysql May 16, 2015
- packets.go: flag clientMultiResults, update status when receiving an
EOF packet, discard additional results on readRow when EOF is reached
- statement.go: currently a nil rows.mc is used as an eof, don’t set it
if there are no columns to avoid that Next() waits indefinitely
- rows.go: discard additional results on close and avoid panic on
Columns()
@julienschmidt
Copy link
Member

@julienschmidt julienschmidt commented May 30, 2015

See #338 and #339

@dsp1589
Copy link

@dsp1589 dsp1589 commented Jan 11, 2016

is this issue resolved? fix has been merged and availble? i am working in big web project and need to implement most in stored procedure. please someone confirm the fix is available. Thanks

@julienschmidt
Copy link
Member

@julienschmidt julienschmidt commented Jan 11, 2016

See #339. It seems to provide a working fix but it has not been merged yet.

@dsp1589
Copy link

@dsp1589 dsp1589 commented Jan 11, 2016

Thanks.is there any other work around to make it work other than cloning and editing source code and building, we dont want to use third party edited code. integration. Any help much appreciated.

@Lord-Y
Copy link

@Lord-Y Lord-Y commented Dec 1, 2018

Hello guys, I know it's been a while since the 1st and last comment. As GO as been evolving, I think a lot of people are waiting for MySQL Stored Procedure Output with IN, INOUT parameters as mentionned @sdming in his 3rd example. So, is it impossible to do?, feasible?

PS: I read this http://go-database-sql.org/surprises.html

@methane
Copy link
Member

@methane methane commented Dec 2, 2018

I don't have never used stored procedure/function. I don't have any interest.
But we support multi statement and multi resultset. Please try it by yourself.

This is issue tracker, not user forum.
Please post only when it's worth enough to bother maintainers.
If your question can be answered from people other than maintainers,
it should be posted on Stackoverflow.

@PaluMacil
Copy link

@PaluMacil PaluMacil commented Apr 4, 2019

Please try it by yourself.
This is issue tracker, not user forum.
Please post only when it's worth enough to bother maintainers.

The rudeness here is impressive!

@methane
Copy link
Member

@methane methane commented Apr 4, 2019

@PaluMacil You must read this article.

Which project do you want to use?

  • Issue tracker is user forum. Maintainer is burned out. Not maintained.
  • Issue tracker is not user forum. Maintainer is not bothered by reading (non issue) questions. So maintainer fixes real issues quickly.
@go-sql-driver go-sql-driver locked as off topic and limited conversation to collaborators Apr 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.