Stored Procedure Multi-Resultset
greg-rychlewski opened this issue · 10 comments
Related to the discussion in #140.
Are you guys interested in returning multi-resultsets? My naive thinking is to return [MyXQL.Result.t()] from the relevant functions.
Yes, I think we should support as many protocol features as possible. However, instead of existing functions returning Result.t | [Result.t], I'd consider introducing new functions like MyXQL.query_many(...) :: [Result.t]. WDYT? cc @josevalim
query_many sounds good to me!
Am I correct that this requires creating query_many/execute_many functions + handle callbacks in DBConnection?
Just want to make sure I don't create a mess before starting the PR.
yeah, we'd need an execute_many too. We could have a prepare_execute_many for completeness, but let's hold off on that one for now. We don't need a new DBConnection callback, we'd change the existing one. Not sure if that's what you were asking.
Oh sorry I meant that MyXQL.execute/MyXQL.query end up calling DBConnection.execute so was curious if you wanted DBConnection.execute_many with a handle_execute_many callback.
But it seems like you are saying we should specify an option that gets passed into DBConnection.execute so that the handle_execute callback can use it to decide if it should keep multiple results or error after the first one. Do I understand correctly?
yeah, let's not change DBConnection, we should be able to do all the work in our callback implementation, the MyXQL.Connection.handle_execute function. You'll probably need to keep some state around in, well, state! (the last argument to all the callbacks)
Will work on the PR :). Thanks!
To be clear, I think the idea is that you send a different query struct to the usual execute/prepare callbacks, and that will lead to a different return type. Postgrex for example has more than struct it sends for querying.
@josevalim I started playing around with it and just wanted to confirm something before I go too far.
Since the new execute_many function will return {:ok, query, result} I'd have to expose the new query struct. I'm not sure how great an idea that is. The best alternative I can think of is to not create the *_many functions and instead add an option to the existing functions. That sounds a bit cleaner to me, but was curious to hear what you guys thought. One of the advantages I see is that Ecto.Adapters.SQL.query would get this new functionality for free.
As a side note:
I wasn't entirely sure execute_many was needed, since I think a query with multiple statements only applies to the text protocol. But I read some more and there is support for preparing and executing stored procedures that return multiple results:
DROP PROCEDURE IF EXISTS multi_procedure;
DELIMITER $$
CREATE PROCEDURE multi_procedure(IN test_param INT)
BEGIN
SELECT 1;
SELECT 2;
END$$
DELIMITER ;
test "with prepared stored procedure", %{client: client} do
{:ok, com_stmt_prepare_ok(statement_id: statement_id)} =
Client.com_stmt_prepare(client, "CALL multi_procedure(?)")
{:ok,
resultset(num_rows: 1, status_flags: status_flags, rows: rows, column_defs: column_defs)} =
Client.com_stmt_execute(client, statement_id, [1], :cursor_type_read_only)
end
produces:
** (MatchError) no match of right hand side value: {:error, :multiple_results}
I think exposing the new struct is ok/expected.