Skip to content

query many#145

Merged
wojtekmach merged 5 commits intoelixir-ecto:masterfrom
greg-rychlewski:query_many
Jan 21, 2022
Merged

query many#145
wojtekmach merged 5 commits intoelixir-ecto:masterfrom
greg-rychlewski:query_many

Conversation

@greg-rychlewski
Copy link
Copy Markdown
Member

@greg-rychlewski greg-rychlewski commented Jan 21, 2022

Closes #143

Introduces new functions query_many, prepare_many, prepare_execute_many and execute_many that allow the user to obtain results for queries that return multiple results, like text queries with multiple statements or stored procedures.

The main idea is:

  • Introduce 2 new structs: MyXQL.Queries and MyXQL.TextQueries
  • Keep a result state in addition to the decoder state while receiving the packets.
    • The result state will keep track of whether this is a single or multiple result and if multiple, store the results already obtained.
    • This lets us decide whether or not to keep going when MySQL says there are more results. And if we are retrieving multiple results to keep the partial results stored somewhere.
    • Keeping this separate from the decoder state means we don't have to copy the partial results through all of the decoder functions. Or to pass the information to determine whether it's a single or multi result query

@greg-rychlewski
Copy link
Copy Markdown
Member Author

sorry I just saw some minor typos in a couple of the tests and forgot to change the error message when raising for multiple results.

Comment thread lib/myxql/connection.ex Outdated
Comment thread lib/myxql/query.ex Outdated
Comment thread lib/myxql/query.ex Outdated
Comment thread lib/myxql/query.ex
end
end

defimpl String.Chars, for: [MyXQL.Query, MyXQL.Queries] do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment thread test/myxql_test.exs Outdated
Comment thread test/myxql_test.exs
assert %MyXQL.Queries{} = query = MyXQL.prepare_many!(c.conn, "", "CALL multi_procedure()")
assert :ok == MyXQL.close(c.conn, query)
end
end
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent coverage!

Comment thread test/myxql_test.exs Outdated
@wojtekmach wojtekmach merged commit 1014f1c into elixir-ecto:master Jan 21, 2022
@wojtekmach
Copy link
Copy Markdown
Member

Thank you!

@greg-rychlewski
Copy link
Copy Markdown
Member Author

greg-rychlewski commented Jan 22, 2022

Would you guys want to add Ecto.Adapters.SQL.query_many/query_many! to ecto_sql? Even though this is the only adapter implementing the function, there might be a benefit if most people are using Ecto vs this driver directly.

I can submit a PR if you think it's a good idea. And I guess return something like {:error, :not_supported} if the adapter is not exporting the appropriate function?

@josevalim
Copy link
Copy Markdown
Member

We could add query_many. It should just raise not supported for the other adapters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stored Procedure Multi-Resultset

3 participants