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

Support for multiple result sets #108

Closed
ekstrom opened this issue May 26, 2020 · 7 comments
Closed

Support for multiple result sets #108

ekstrom opened this issue May 26, 2020 · 7 comments

Comments

@ekstrom
Copy link

ekstrom commented May 26, 2020

It would be useful to have support for multiple result sets.

Example code:

query = "SELECT 1 AS 'first_result_set'; SELECT 2 AS 'second_result_set';"
Tds.query!(pid, query, [])

Expected result:

[%Tds.Result{columns: ["first_result_set"], num_rows: 1, rows: [[1]]}, %Tds.Result{columns: ["second_result_set"], num_rows: 1, rows: [[2]]}]

Actual result:

%Tds.Result{columns: ["first_result_set"], num_rows: 1, rows: [[1]]}
@mjaric
Copy link
Member

mjaric commented May 28, 2020

Last year I refactored decoder part, it reads and keeps all results until it must hadoff result to user code. Reason is that ecto expects only single result, so Tds.Protocol pulls out last result here.

Since ecto adapter does not use Tds.query/3 nor Tds.query/4 it is possible to pass additional flag in options that should tell internally Tds.Protocol module to rather return all results. For instance, here it could be something like:

  def query(pid, statement, params, opts \\ []) do
    query = %Query{statement: statement}
    opts = 
      opts
      |> Keyword.put_new(:parameters, params)
      |> Keyword.put(:resultset, true) # <-----   THE FLAG!

    case DBConnection.prepare_execute(pid, query, params, opts) do
      {:ok, _query, result} -> {:ok, result}
      {:error, err} -> {:error, err}
    end
  end

Then it could pack among all results additional info, like messages server returned (info, error ...), num_results, and result set count.

It is worth mentioning that results are in reverse order so they need to be Enum.reversed before they are handed to user code.

@mjaric
Copy link
Member

mjaric commented May 28, 2020

One more note, since unit tests also expect single Tds.Result, all must be refactored to pass with this change

@josevalim
Copy link
Member

Perhaps the best option is to introduce a separate function, such as query_multi or query_many or similar. So users can explicitly opt-in into it.

mjaric added a commit that referenced this issue Jun 3, 2020
this should solve issue #108
@mjaric
Copy link
Member

mjaric commented Jun 3, 2020

@josevalim, @ekstrom , I think this is going to work for any multi batch script for MSSQL Server. It even follows inserts without output. We were lucky I refactored decoder last winter so this was easy to expose

@mjaric
Copy link
Member

mjaric commented Jun 3, 2020

@ekstrom BTW, please point your fork to tds master and run test, but there shouldn't be anything broken

@ekstrom
Copy link
Author

ekstrom commented Jun 14, 2020

Tested it and it works as expected. Thanks for the good work 😄

@mjaric
Copy link
Member

mjaric commented Jun 14, 2020

Released in v2.1.1.

Thank you!

@mjaric mjaric closed this as completed Jun 14, 2020
leandrocp added a commit to leandrocp/ecto_sql that referenced this issue Sep 26, 2020
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

No branches or pull requests

3 participants