From 1bb3ab5ee27260ffadfb9f04a440c4024788667d Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Tue, 8 Mar 2022 00:14:30 -0500 Subject: [PATCH 1/6] fixes --- lib/myxql.ex | 38 ++++++++++- lib/myxql/client.ex | 31 +++------ lib/myxql/connection.ex | 127 +++++++++++++++++-------------------- lib/myxql/protocol.ex | 40 ++++++------ test/myxql/client_test.exs | 14 ++-- test/myxql_test.exs | 106 +++++++++++++++++++++++-------- test/test_helper.exs | 20 +++++- 7 files changed, 226 insertions(+), 150 deletions(-) diff --git a/lib/myxql.ex b/lib/myxql.ex index 07f7048..0ef8461 100644 --- a/lib/myxql.ex +++ b/lib/myxql.ex @@ -211,6 +211,14 @@ defmodule MyXQL do * requires two roundtrips to the DB server: one for preparing the statement and one for executing it. This can be alleviated by holding on to prepared statement and executing it multiple times. + ## Stored procedures + + A successfully executed stored procedure always returns the affected row count + of the last statement. This means any stored procedure containing statements that + return result sets, such as `SELECT` statements, must use `query_many/4` and + similar functions. These functions will return one result for each statement + returning a result set and one for the affected row count of the last statement. + ## Options * `:query_type` - use `:binary` for binary protocol (prepared statements), `:binary_then_text` to attempt @@ -365,6 +373,14 @@ defmodule MyXQL do but the statement isn't. If a new statement is given to an old name, the old statement will be the one effectively used. + ## Stored procedures + + A successfully executed stored procedure always returns the affected row count + of the last statement. This means any stored procedure containing statements that + return result sets, such as `SELECT` statements, must use `prepare_many/4` and similar + functions. These functions will return one result for each statement returning a + result set and one for the affected row count of the last statement. + ## Options Options are passed to `DBConnection.prepare/3`, see it's documentation for @@ -417,7 +433,7 @@ defmodule MyXQL do ## Examples iex> {:ok, query} = MyXQL.prepare_many(conn, "", "CALL multi_procedure()") - iex> {:ok, [%MyXQL.Result{rows: [row1]}, %MyXQL.Result{rows: [row2]}]} = MyXQL.execute_many(conn, query, [2, 3]) + iex> {:ok, [%MyXQL.Result{rows: [row1]}, %MyXQL.Result{rows: [row2]}, %MyXQL.Result{rows: nil}]} = MyXQL.execute_many(conn, query, [2, 3]) iex> row1 [2] iex> row2 @@ -449,6 +465,14 @@ defmodule MyXQL do @doc """ Prepares and executes a query that returns a single result, in a single step. + ## Stored procedures + + A successfully executed stored procedure always returns the affected row count + of the last statement. This means any stored procedure containing statements that + return result sets, such as `SELECT` statements, must use `prepare_execute_many/5` + and similar functions. These functions will return one result for each statement + returning a result set and one for the affected row count of the last statement. + ## Options Options are passed to `DBConnection.prepare_execute/4`, see it's documentation for @@ -499,7 +523,7 @@ defmodule MyXQL do ## Examples - iex> {:ok, _, [%MyXQL.Result{rows: [row1]}, %MyXQL.Result{rows: [row2]}]} = MyXQL.prepare_execute(conn, "", "CALL multi_procedure()") + iex> {:ok, _, [%MyXQL.Result{rows: [row1]}, %MyXQL.Result{rows: [row2]}, %MyXQL.Result{rows: nil}]} = MyXQL.prepare_execute(conn, "", "CALL multi_procedure()") iex> row1 [2] iex> row2 @@ -533,6 +557,14 @@ defmodule MyXQL do @doc """ Executes a prepared query that returns a single result. + ## Stored procedures + + A successfully executed stored procedure always returns the affected row count + of the last statement. This means any stored procedure containing statements that + return result sets, such as `SELECT` statements, must use execute_many/4 and + similar functions. These functions will return one result for each statement + returning a result set and one for the affected row count of the last statement. + ## Options Options are passed to `DBConnection.execute/4`, see it's documentation for @@ -575,7 +607,7 @@ defmodule MyXQL do ## Examples iex> {:ok, query} = MyXQL.prepare_many(conn, "", "CALL multi_procedure()") - iex> {:ok, [%MyXQL.Result{rows: [row1]}, %MyXQL.Result{rows: [row2]}]} = MyXQL.execute_many(conn, query) + iex> {:ok, [%MyXQL.Result{rows: [row1]}, %MyXQL.Result{rows: [row2]}, %MyXQL.Result{rows: nil}]} = MyXQL.execute_many(conn, query) iex> row1 [2] iex> row2 diff --git a/lib/myxql/client.ex b/lib/myxql/client.ex index e973f6e..e82ddc4 100644 --- a/lib/myxql/client.ex +++ b/lib/myxql/client.ex @@ -192,28 +192,6 @@ defmodule MyXQL.Client do defp recv_packets(data, decode, decoder_state, result_state, timeout, client, partial \\ <<>>) - defp recv_packets( - <> = data, - decoder, - {:more_results, resultset}, - result_state, - timeout, - client, - partial - ) - when size < @default_max_packet_size do - case decode_more_results(<>, rest, resultset, result_state) do - {:cont, decoder_state, result_state} -> - recv_packets(data, decoder, decoder_state, result_state, timeout, client, partial) - - {:halt, result} -> - {:ok, result} - - {:error, _} = error -> - error - end - end - defp recv_packets( <>, decoder, @@ -225,6 +203,15 @@ defmodule MyXQL.Client do ) when size < @default_max_packet_size do case decoder.(<>, rest, decoder_state) do + {:cont, {:more_results, result}} -> + case result_state do + :single -> + {:error, :multiple_results} + + {:many, results} -> + recv_packets(rest, decoder, :initial, {:many, [result | results]}, timeout, client) + end + {:cont, decoder_state} -> recv_packets(rest, decoder, decoder_state, result_state, timeout, client) diff --git a/lib/myxql/connection.ex b/lib/myxql/connection.ex index 86bd590..fc65c1d 100644 --- a/lib/myxql/connection.ex +++ b/lib/myxql/connection.ex @@ -272,80 +272,32 @@ defmodule MyXQL.Connection do ## Internals - defp result( - {:ok, - ok_packet( - last_insert_id: last_insert_id, - affected_rows: affected_rows, - status_flags: status_flags, - num_warnings: num_warnings - )}, - query, - state - ) do - result = %Result{ - connection_id: state.client.connection_id, - last_insert_id: last_insert_id, - num_rows: affected_rows, - num_warnings: num_warnings - } - - {:ok, query, result, put_status(state, status_flags)} + defp result({:ok, ok_packet(status_flags: status_flags) = result}, query, state) do + {:ok, query, format_result(result, state), put_status(state, status_flags)} end - defp result( - {:ok, - resultset( - column_defs: column_defs, - num_rows: num_rows, - rows: rows, - status_flags: status_flags, - num_warnings: num_warnings - )}, - query, - state - ) do - columns = Enum.map(column_defs, &elem(&1, 1)) - - result = %Result{ - connection_id: state.client.connection_id, - columns: columns, - num_rows: num_rows, - rows: rows, - num_warnings: num_warnings - } + defp result({:ok, resultset(status_flags: status_flags) = result}, query, state) do + {:ok, query, format_result(result, state), put_status(state, status_flags)} + end - {:ok, query, result, put_status(state, status_flags)} + # If a multi-result query has an error, it will be the latest query executed. + # The results are returned to this function in reverse order so it's the first + # in the result list. + defp result({:ok, [err_packet() = result | _rest]}, query, state) do + result({:ok, result}, query, state) end - defp result({:ok, resultsets}, query, state) when is_list(resultsets) do + defp result({:ok, results}, query, state) when is_list(results) do {results, status_flags} = - Enum.reduce(resultsets, {[], nil}, fn resultset, {results, newest_status_flags} -> - resultset( - column_defs: column_defs, - num_rows: num_rows, - rows: rows, - status_flags: status_flags, - num_warnings: num_warnings - ) = resultset - - columns = Enum.map(column_defs, &elem(&1, 1)) - - result = %Result{ - connection_id: state.client.connection_id, - columns: columns, - num_rows: num_rows, - rows: rows, - num_warnings: num_warnings - } - - # Keep status flags from the last query. The resultsets - # are given to this function in reverse order, so it is the first one. - if newest_status_flags do - {[result | results], newest_status_flags} - else - {[result | results], status_flags} - end + Enum.reduce(results, {[], nil}, fn + result, {results, latest_status_flags} -> + # Keep status flags from the last query. The result sets + # are given to this function in reverse order, so it is the first one. + if latest_status_flags do + {[format_result(result, state) | results], latest_status_flags} + else + {[format_result(result, state) | results], status_flags(result)} + end end) {:ok, query, results, put_status(state, status_flags)} @@ -374,6 +326,45 @@ defmodule MyXQL.Connection do {:error, %DBConnection.ConnectionError{message: message}} end + defp format_result( + ok_packet( + last_insert_id: last_insert_id, + affected_rows: affected_rows, + num_warnings: num_warnings + ), + state + ) do + %Result{ + connection_id: state.client.connection_id, + last_insert_id: last_insert_id, + num_rows: affected_rows, + num_warnings: num_warnings + } + end + + defp format_result( + resultset( + column_defs: column_defs, + num_rows: num_rows, + rows: rows, + num_warnings: num_warnings + ), + state + ) do + columns = Enum.map(column_defs, &elem(&1, 1)) + + %Result{ + connection_id: state.client.connection_id, + columns: columns, + num_rows: num_rows, + rows: rows, + num_warnings: num_warnings + } + end + + defp status_flags(ok_packet(status_flags: status_flags)), do: status_flags + defp status_flags(resultset(status_flags: status_flags)), do: status_flags + defp error(reason, %{statement: statement}, state) do error(reason, statement, state) end diff --git a/lib/myxql/protocol.ex b/lib/myxql/protocol.ex index 8f67076..50f8294 100644 --- a/lib/myxql/protocol.ex +++ b/lib/myxql/protocol.ex @@ -316,11 +316,27 @@ defmodule MyXQL.Protocol do end # https://dev.mysql.com/doc/internals/en/com-query-response.html#packet-COM_QUERY_Response + # If an ok packet (0x00) is received during the `:initial` state it can be done of two things: + # 1. The result of a single statement query which doesn't return a result set (i.e `CREATE`) + # 2. The partial result of a multi-statement query where the current statement being executed + # doesn't return a result set. + # If the first scenario, return the ok packet as the response. + # If the second scenario, store the result and keep reading from the socket. def decode_com_query_response(<<0x00, rest::binary>>, "", :initial) do - {:halt, decode_ok_packet_body(rest)} + ok_packet(status_flags: status_flags) = ok_response = decode_ok_packet_body(rest) + + if has_status_flag?(status_flags, :server_more_results_exists) do + {:cont, {:more_results, ok_response}} + else + {:halt, ok_response} + end + end + + def decode_com_query_response(<<0x00, rest::binary>>, _next_data, :initial) do + {:cont, {:more_results, decode_ok_packet_body(rest)}} end - def decode_com_query_response(<<0xFF, rest::binary>>, "", :initial) do + def decode_com_query_response(<<0xFF, rest::binary>>, _next_data, :initial) do {:halt, decode_err_packet_body(rest)} end @@ -482,26 +498,6 @@ defmodule MyXQL.Protocol do ) end - def decode_more_results(payload, "", resultset, result_state) do - ok_packet(status_flags: status_flags) = decode_generic_response(payload) - - case result_state do - :single -> - {:halt, resultset(resultset, status_flags: status_flags)} - - {:many, results} -> - {:halt, [resultset(resultset, status_flags: status_flags) | results]} - end - end - - def decode_more_results(_payload, _next_data, _resultset, :single) do - {:error, :multiple_results} - end - - def decode_more_results(_payload, _next_data, resultset, {:many, results}) do - {:cont, :initial, {:many, [resultset | results]}} - end - defp decode_resultset(payload, _next_data, :initial, _row_decoder) do {:cont, {:column_defs, decode_int_lenenc(payload), []}} end diff --git a/test/myxql/client_test.exs b/test/myxql/client_test.exs index 22af36b..001824d 100644 --- a/test/myxql/client_test.exs +++ b/test/myxql/client_test.exs @@ -348,9 +348,9 @@ defmodule MyXQL.ClientTest do test "with stored procedure of single result", %{client: client} do {:ok, com_stmt_prepare_ok(statement_id: statement_id)} = - Client.com_stmt_prepare(client, "CALL single_procedure()") + Client.com_stmt_prepare(client, "CALL single_ok_procedure()") - {:ok, resultset(num_rows: 1, status_flags: status_flags)} = + {:ok, ok_packet(affected_rows: 1, status_flags: status_flags)} = Client.com_stmt_execute(client, statement_id, [], :cursor_type_read_only) assert list_status_flags(status_flags) == [:server_status_autocommit] @@ -359,7 +359,7 @@ defmodule MyXQL.ClientTest do test "with stored procedure of multiple results", %{client: client} do {:ok, com_stmt_prepare_ok(statement_id: statement_id)} = - Client.com_stmt_prepare(client, "CALL multi_procedure()") + Client.com_stmt_prepare(client, "CALL one_resultset_one_ok_procedure()") assert {:error, :multiple_results} = Client.com_stmt_execute(client, statement_id, [], :cursor_type_read_only) @@ -371,12 +371,12 @@ defmodule MyXQL.ClientTest do {:ok, com_stmt_prepare_ok(statement_id: statement_id)} = Client.com_stmt_prepare(client, "CALL cursor_procedure()") - {:ok, resultset(num_rows: 1, rows: [[3]])} = - Client.com_stmt_execute(client, statement_id, [], :cursor_type_read_only) + {:ok, [ok_packet(affected_rows: 0), resultset(num_rows: 1, rows: [[3]])]} = + Client.com_stmt_execute(client, statement_id, [], :cursor_type_read_only, {:many, []}) # This will be called if, for instance, someone issues the procedure statement from Ecto.Adapters.SQL.query - {:ok, resultset(num_rows: 1, rows: [[3]])} = - Client.com_stmt_execute(client, statement_id, [], :cursor_type_no_cursor) + {:ok, [ok_packet(affected_rows: 0), resultset(num_rows: 1, rows: [[3]])]} = + Client.com_stmt_execute(client, statement_id, [], :cursor_type_no_cursor, {:many, []}) Client.com_quit(client) end diff --git a/test/myxql_test.exs b/test/myxql_test.exs index 84d0e66..04ff4fa 100644 --- a/test/myxql_test.exs +++ b/test/myxql_test.exs @@ -184,6 +184,19 @@ defmodule MyXQLTest do assert {:ok, [%MyXQL.Result{rows: [[1]]}]} = MyXQL.query_many(c.conn, "SELECT 1;", [], query_type: :text) + + assert {:ok, [%MyXQL.Result{num_rows: 0, rows: nil}]} = + MyXQL.query_many(c.conn, "DROP TABLE IF EXISTS not_a_table;", [], query_type: :text) + + assert {:ok, [%MyXQL.Result{num_rows: 0, rows: nil}, %MyXQL.Result{rows: [[1]]}]} = + MyXQL.query_many(c.conn, "DROP TABLE IF EXISTS not_a_table; SELECT 1;", [], + query_type: :text + ) + + assert {:ok, [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{num_rows: 0, rows: nil}]} = + MyXQL.query_many(c.conn, "SELECT 1; DROP TABLE IF EXISTS not_a_table;", [], + query_type: :text + ) end test "query_many!/4 with text", c do @@ -192,6 +205,32 @@ defmodule MyXQLTest do assert [%MyXQL.Result{rows: [[1]]}] = MyXQL.query_many!(c.conn, "SELECT 1;", [], query_type: :text) + + assert [%MyXQL.Result{num_rows: 0, rows: nil}] = + MyXQL.query_many!(c.conn, "DROP TABLE IF EXISTS not_a_table;", [], + query_type: :text + ) + + assert [%MyXQL.Result{num_rows: 0, rows: nil}, %MyXQL.Result{rows: [[1]]}] = + MyXQL.query_many!(c.conn, "DROP TABLE IF EXISTS not_a_table; SELECT 1;", [], + query_type: :text + ) + + assert [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{num_rows: 0, rows: nil}] = + MyXQL.query_many!(c.conn, "SELECT 1; DROP TABLE IF EXISTS not_a_table;", [], + query_type: :text + ) + end + + test "query_many/4 with text returning error", c do + assert {:error, %MyXQL.Error{mysql: %{code: 1064}}} = + MyXQL.query_many(c.conn, "SELECT 1; BADCOMMAND;", [], query_type: :text) + end + + test "query_many!/4 with text returning error", c do + assert_raise MyXQL.Error, ~r"\(1064\)", fn -> + MyXQL.query_many!(c.conn, "SELECT 1; BADCOMMAND;", [], query_type: :text) + end end end @@ -624,46 +663,54 @@ defmodule MyXQLTest do setup :connect test "text query", c do - assert %MyXQL.Result{rows: [[1]]} = - MyXQL.query!(c.conn, "CALL single_procedure()", [], query_type: :text) + assert %MyXQL.Result{num_rows: 1, rows: nil} = + MyXQL.query!(c.conn, "CALL single_ok_procedure()", [], query_type: :text) - assert %MyXQL.Result{rows: [[1]]} = - MyXQL.query!(c.conn, "CALL single_procedure()", [], query_type: :text) + assert %MyXQL.Result{num_rows: 1, rows: nil} = + MyXQL.query!(c.conn, "CALL single_ok_procedure()", [], query_type: :text) - assert [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{rows: [[2]]}] = - MyXQL.query_many!(c.conn, "CALL multi_procedure()") + assert [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{num_rows: 0, rows: nil}] = + MyXQL.query_many!(c.conn, "CALL one_resultset_one_ok_procedure()") + + assert [ + %MyXQL.Result{rows: [[1]]}, + %MyXQL.Result{rows: [[2]]}, + %MyXQL.Result{num_rows: 0, rows: nil} + ] = MyXQL.query_many!(c.conn, "CALL multi_procedure()") end test "prepared query", c do - assert {%MyXQL.Query{}, %MyXQL.Result{rows: [[1]]}} = - MyXQL.prepare_execute!(c.conn, "", "CALL single_procedure()") + assert {%MyXQL.Query{}, %MyXQL.Result{num_rows: 1, rows: nil}} = + MyXQL.prepare_execute!(c.conn, "", "CALL single_ok_procedure()") - assert {%MyXQL.Query{}, %MyXQL.Result{rows: [[1]]}} = - MyXQL.prepare_execute!(c.conn, "", "CALL single_procedure()") + assert {%MyXQL.Queries{}, + [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{num_rows: 0, rows: nil}]} = + MyXQL.prepare_execute_many!(c.conn, "", "CALL one_resultset_one_ok_procedure()") - assert {%MyXQL.Queries{}, [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{rows: [[2]]}]} = - MyXQL.prepare_execute_many!(c.conn, "", "CALL multi_procedure()") + assert {%MyXQL.Queries{}, + [ + %MyXQL.Result{rows: [[1]]}, + %MyXQL.Result{rows: [[2]]}, + %MyXQL.Result{num_rows: 0, rows: nil} + ]} = MyXQL.prepare_execute_many!(c.conn, "", "CALL multi_procedure()") - assert %MyXQL.Queries{} = query = MyXQL.prepare_many!(c.conn, "", "CALL multi_procedure()") + assert %MyXQL.Queries{} = + query = MyXQL.prepare_many!(c.conn, "", "CALL one_resultset_one_ok_procedure()") - assert [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{rows: [[2]]}] = + assert [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{num_rows: 0, rows: nil}] = MyXQL.execute_many!(c.conn, query) - end - test "stream procedure with single result", c do - statement = "CALL single_procedure()" - - {:ok, result} = - MyXQL.transaction(c.conn, fn conn -> - stream = MyXQL.stream(conn, statement, [], max_rows: 2) - Enum.to_list(stream) - end) + assert %MyXQL.Queries{} = query = MyXQL.prepare_many!(c.conn, "", "CALL multi_procedure()") - assert [%MyXQL.Result{rows: [[1]]}] = result + assert [ + %MyXQL.Result{rows: [[1]]}, + %MyXQL.Result{rows: [[2]]}, + %MyXQL.Result{num_rows: 0, rows: nil} + ] = MyXQL.execute_many!(c.conn, query) end test "stream procedure with multiple results", c do - statement = "CALL multi_procedure()" + statement = "CALL one_resultset_one_ok_procedure()" assert_raise RuntimeError, ~r"returning multiple results is not supported", fn -> MyXQL.transaction(c.conn, fn conn -> @@ -694,6 +741,13 @@ defmodule MyXQLTest do end test "using execute/4 with a multiple result query", c do + %MyXQL.Queries{} = + query = MyXQL.prepare_many!(c.conn, "", "CALL one_resultset_one_ok_procedure()") + + assert_raise FunctionClauseError, fn -> + MyXQL.execute(c.conn, query) + end + %MyXQL.Queries{} = query = MyXQL.prepare_many!(c.conn, "", "CALL multi_procedure()") assert_raise FunctionClauseError, fn -> @@ -728,7 +782,7 @@ defmodule MyXQLTest do end test "using execute_many/4 with a single result query", c do - %MyXQL.Query{} = query = MyXQL.prepare!(c.conn, "", "CALL single_procedure()") + %MyXQL.Query{} = query = MyXQL.prepare!(c.conn, "", "CALL single_ok_procedure()") assert_raise FunctionClauseError, fn -> MyXQL.execute_many(c.conn, query) diff --git a/test/test_helper.exs b/test/test_helper.exs index 9370cc3..ddbf26f 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -140,9 +140,25 @@ defmodule TestHelper do my_char CHAR ); - DROP PROCEDURE IF EXISTS single_procedure; + # This will only return the trailing ok_packet + # because the commands inside the stored procedure + # do not return result sets. + DROP PROCEDURE IF EXISTS single_ok_procedure; DELIMITER $$ - CREATE PROCEDURE single_procedure() + CREATE PROCEDURE single_ok_procedure() + BEGIN + CREATE TABLE IF NOT EXISTS integers (x int); + INSERT INTO integers VALUES (10); + INSERT INTO integers VALUES (11); + END$$ + DELIMITER ; + + # This will return a single result set + # and a single trailing ok packet. It must + # be used with the query_many variants. + DROP PROCEDURE IF EXISTS one_resultset_one_ok_procedure; + DELIMITER $$ + CREATE PROCEDURE one_resultset_one_ok_procedure() BEGIN SELECT 1; END$$ From ff861cbb5486fb4a1d2de9720cdb9319e67edd32 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Tue, 8 Mar 2022 00:58:17 -0500 Subject: [PATCH 2/6] Mariadb counts affected rows in trailing ok packets differently than mysql --- test/myxql/client_test.exs | 2 +- test/myxql_test.exs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/myxql/client_test.exs b/test/myxql/client_test.exs index 001824d..f8e0a76 100644 --- a/test/myxql/client_test.exs +++ b/test/myxql/client_test.exs @@ -350,7 +350,7 @@ defmodule MyXQL.ClientTest do {:ok, com_stmt_prepare_ok(statement_id: statement_id)} = Client.com_stmt_prepare(client, "CALL single_ok_procedure()") - {:ok, ok_packet(affected_rows: 1, status_flags: status_flags)} = + {:ok, ok_packet(status_flags: status_flags)} = Client.com_stmt_execute(client, statement_id, [], :cursor_type_read_only) assert list_status_flags(status_flags) == [:server_status_autocommit] diff --git a/test/myxql_test.exs b/test/myxql_test.exs index 04ff4fa..96546c7 100644 --- a/test/myxql_test.exs +++ b/test/myxql_test.exs @@ -663,10 +663,10 @@ defmodule MyXQLTest do setup :connect test "text query", c do - assert %MyXQL.Result{num_rows: 1, rows: nil} = + assert %MyXQL.Result{rows: nil} = MyXQL.query!(c.conn, "CALL single_ok_procedure()", [], query_type: :text) - assert %MyXQL.Result{num_rows: 1, rows: nil} = + assert %MyXQL.Result{rows: nil} = MyXQL.query!(c.conn, "CALL single_ok_procedure()", [], query_type: :text) assert [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{num_rows: 0, rows: nil}] = @@ -680,7 +680,7 @@ defmodule MyXQLTest do end test "prepared query", c do - assert {%MyXQL.Query{}, %MyXQL.Result{num_rows: 1, rows: nil}} = + assert {%MyXQL.Query{}, %MyXQL.Result{rows: nil}} = MyXQL.prepare_execute!(c.conn, "", "CALL single_ok_procedure()") assert {%MyXQL.Queries{}, From f04be981de54da404290875ceb1fecea126411da Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Wed, 16 Mar 2022 00:10:09 -0400 Subject: [PATCH 3/6] review comments --- lib/myxql/connection.ex | 12 +++++++++-- test/myxql_test.exs | 47 ++++++++++++++++------------------------- test/test_helper.exs | 6 ++++-- 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/lib/myxql/connection.ex b/lib/myxql/connection.ex index fc65c1d..d33b9f9 100644 --- a/lib/myxql/connection.ex +++ b/lib/myxql/connection.ex @@ -225,7 +225,7 @@ defmodule MyXQL.Connection do end other -> - result(other, query, state) + stream_result(other, query, state) end end @@ -245,7 +245,7 @@ defmodule MyXQL.Connection do end other -> - result(other, query, state) + stream_result(other, query, state) end end @@ -271,6 +271,14 @@ defmodule MyXQL.Connection do end ## Internals + defp stream_result({:error, :multiple_results}, _query, _state) do + raise RuntimeError, + "streaming stored procedures is not supported. Use MyXQL.query_many/4 and similar functions." + end + + defp stream_result(result, query, state) do + result(result, query, state) + end defp result({:ok, ok_packet(status_flags: status_flags) = result}, query, state) do {:ok, query, format_result(result, state), put_status(state, status_flags)} diff --git a/test/myxql_test.exs b/test/myxql_test.exs index 96546c7..a4b278a 100644 --- a/test/myxql_test.exs +++ b/test/myxql_test.exs @@ -199,29 +199,6 @@ defmodule MyXQLTest do ) end - test "query_many!/4 with text", c do - assert [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{rows: [[2]]}] = - MyXQL.query_many!(c.conn, "SELECT 1; SELECT 2", [], query_type: :text) - - assert [%MyXQL.Result{rows: [[1]]}] = - MyXQL.query_many!(c.conn, "SELECT 1;", [], query_type: :text) - - assert [%MyXQL.Result{num_rows: 0, rows: nil}] = - MyXQL.query_many!(c.conn, "DROP TABLE IF EXISTS not_a_table;", [], - query_type: :text - ) - - assert [%MyXQL.Result{num_rows: 0, rows: nil}, %MyXQL.Result{rows: [[1]]}] = - MyXQL.query_many!(c.conn, "DROP TABLE IF EXISTS not_a_table; SELECT 1;", [], - query_type: :text - ) - - assert [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{num_rows: 0, rows: nil}] = - MyXQL.query_many!(c.conn, "SELECT 1; DROP TABLE IF EXISTS not_a_table;", [], - query_type: :text - ) - end - test "query_many/4 with text returning error", c do assert {:error, %MyXQL.Error{mysql: %{code: 1064}}} = MyXQL.query_many(c.conn, "SELECT 1; BADCOMMAND;", [], query_type: :text) @@ -666,9 +643,6 @@ defmodule MyXQLTest do assert %MyXQL.Result{rows: nil} = MyXQL.query!(c.conn, "CALL single_ok_procedure()", [], query_type: :text) - assert %MyXQL.Result{rows: nil} = - MyXQL.query!(c.conn, "CALL single_ok_procedure()", [], query_type: :text) - assert [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{num_rows: 0, rows: nil}] = MyXQL.query_many!(c.conn, "CALL one_resultset_one_ok_procedure()") @@ -694,8 +668,12 @@ defmodule MyXQLTest do %MyXQL.Result{num_rows: 0, rows: nil} ]} = MyXQL.prepare_execute_many!(c.conn, "", "CALL multi_procedure()") + assert %MyXQL.Query{} = query = MyXQL.prepare!(c.conn, "", "CALL single_ok_procedure()") + + assert %MyXQL.Result{rows: nil} = MyXQL.execute!(c.conn, query) + assert %MyXQL.Queries{} = - query = MyXQL.prepare_many!(c.conn, "", "CALL one_resultset_one_ok_procedure()") + query = MyXQL.prepare_many!(c.conn, "", "CALL one_resultset_one_ok_procedure()") assert [%MyXQL.Result{rows: [[1]]}, %MyXQL.Result{num_rows: 0, rows: nil}] = MyXQL.execute_many!(c.conn, query) @@ -709,10 +687,10 @@ defmodule MyXQLTest do ] = MyXQL.execute_many!(c.conn, query) end - test "stream procedure with multiple results", c do + test "stream stored procedure", c do statement = "CALL one_resultset_one_ok_procedure()" - assert_raise RuntimeError, ~r"returning multiple results is not supported", fn -> + assert_raise RuntimeError, ~r"streaming stored procedures is not supported", fn -> MyXQL.transaction(c.conn, fn conn -> stream = MyXQL.stream(conn, statement, [], max_rows: 2) Enum.to_list(stream) @@ -793,6 +771,17 @@ defmodule MyXQLTest do assert %MyXQL.Queries{} = query = MyXQL.prepare_many!(c.conn, "", "CALL multi_procedure()") assert :ok == MyXQL.close(c.conn, query) end + + test "using stream/4 with a multiple result query that is not a stored procedure", c do + statement = "SELECT 1; SELECT 2;" + + assert_raise MyXQL.Error, ~r"\(1064\)", fn -> + MyXQL.transaction(c.conn, fn conn -> + stream = MyXQL.stream(conn, statement, [], max_rows: 2) + Enum.to_list(stream) + end) + end + end end @tag :skip diff --git a/test/test_helper.exs b/test/test_helper.exs index ddbf26f..6c4dd11 100644 --- a/test/test_helper.exs +++ b/test/test_helper.exs @@ -140,9 +140,11 @@ defmodule TestHelper do my_char CHAR ); - # This will only return the trailing ok_packet + # This will only return the trailing ok packet # because the commands inside the stored procedure - # do not return result sets. + # do not return result sets. Commands inside of stored + # procedures that return ok packets do not have their + # results sent through the wire. DROP PROCEDURE IF EXISTS single_ok_procedure; DELIMITER $$ CREATE PROCEDURE single_ok_procedure() From fd904eb959922a807b3fd3bb8c8ca62ff4f08046 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Wed, 16 Mar 2022 22:48:14 -0400 Subject: [PATCH 4/6] improve comments --- lib/myxql/connection.ex | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/myxql/connection.ex b/lib/myxql/connection.ex index d33b9f9..dadba16 100644 --- a/lib/myxql/connection.ex +++ b/lib/myxql/connection.ex @@ -271,6 +271,10 @@ defmodule MyXQL.Connection do end ## Internals + + # This error can only be received when attempting to stream a stored procedure. + # Attemping to stream a text query with multiple-statements separated by a semi-colon + # will return a syntax error. defp stream_result({:error, :multiple_results}, _query, _state) do raise RuntimeError, "streaming stored procedures is not supported. Use MyXQL.query_many/4 and similar functions." @@ -288,9 +292,8 @@ defmodule MyXQL.Connection do {:ok, query, format_result(result, state), put_status(state, status_flags)} end - # If a multi-result query has an error, it will be the latest query executed. - # The results are returned to this function in reverse order so it's the first - # in the result list. + # Receiving an error result from a multi-statement query will halt the rest of its processing. + # The results are given to this function in reverse order, so it is the first one. defp result({:ok, [err_packet() = result | _rest]}, query, state) do result({:ok, result}, query, state) end @@ -299,8 +302,8 @@ defmodule MyXQL.Connection do {results, status_flags} = Enum.reduce(results, {[], nil}, fn result, {results, latest_status_flags} -> - # Keep status flags from the last query. The result sets - # are given to this function in reverse order, so it is the first one. + # Keep status flags from the last query. The results are given + # this function in reverse order, so it is the first one. if latest_status_flags do {[format_result(result, state) | results], latest_status_flags} else From 79c027927d6ff8997fa6729ad2806c078520ad7a Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Wed, 16 Mar 2022 23:38:42 -0400 Subject: [PATCH 5/6] improve stored procedure docs --- lib/myxql.ex | 156 ++++++++++++++++++++++++++++++++++------ lib/myxql/connection.ex | 4 +- 2 files changed, 138 insertions(+), 22 deletions(-) diff --git a/lib/myxql.ex b/lib/myxql.ex index 0ef8461..e8bc127 100644 --- a/lib/myxql.ex +++ b/lib/myxql.ex @@ -213,11 +213,40 @@ defmodule MyXQL do ## Stored procedures - A successfully executed stored procedure always returns the affected row count - of the last statement. This means any stored procedure containing statements that - return result sets, such as `SELECT` statements, must use `query_many/4` and - similar functions. These functions will return one result for each statement - returning a result set and one for the affected row count of the last statement. + A stored procedure containing statements that return rows, such as `SELECT` + statements, must use `query_many/4` and similar functions. This is because + stored procedures always return a final result with the number of rows affected + by the last statement. + + Take, for example, the following stored procedure: + + DELIMITER $$ + CREATE PROCEDURE stored_procedure() + BEGIN + SELECT 1; + END$$ + DELIMITER ; + + There will be one result for the `SELECT 1` statement and another result + stating that 0 rows were affected by the last statement. + + In contrast to this, a stored procedure that doesn't contain row-returning + statements can use this function. In this scenario, only the final result + with the number of rows affected by the last statment will be returned. + + Take, for example, the following stored procedure: + + DELIMITER $$ + CREATE PROCEDURE stored_procedure() + BEGIN + CREATE TABLE IF NOT EXISTS integers (x int); + INSERT INTO integers VALUES (10); + END$$ + DELIMITER ; + + Because `CREATE` and `INSERT` statements do not return rows, there will be a + single result stating that 1 row was affected by the last statement. This is + referencing the row that was inserted. ## Options @@ -375,11 +404,40 @@ defmodule MyXQL do ## Stored procedures - A successfully executed stored procedure always returns the affected row count - of the last statement. This means any stored procedure containing statements that - return result sets, such as `SELECT` statements, must use `prepare_many/4` and similar - functions. These functions will return one result for each statement returning a - result set and one for the affected row count of the last statement. + A stored procedure containing statements that return rows, such as `SELECT` + statements, must use `prepare_many/4` and similar functions. This is because + stored procedures always return a final result with the number of rows affected + by the last statement. + + Take, for example, the following stored procedure: + + DELIMITER $$ + CREATE PROCEDURE stored_procedure() + BEGIN + SELECT 1; + END$$ + DELIMITER ; + + There will be one result for the `SELECT 1` statement and another result + stating that 0 rows were affected by the last statement. + + In contrast to this, a stored procedure that doesn't contain row-returning + statements can use this function. In this scenario, only the final result + with the number of rows affected by the last statment will be returned. + + Take, for example, the following stored procedure: + + DELIMITER $$ + CREATE PROCEDURE stored_procedure() + BEGIN + CREATE TABLE IF NOT EXISTS integers (x int); + INSERT INTO integers VALUES (10); + END$$ + DELIMITER ; + + Because `CREATE` and `INSERT` statements do not return rows, there will be a + single result stating that 1 row was affected by the last statement. This is + referencing the row that was inserted. ## Options @@ -467,11 +525,40 @@ defmodule MyXQL do ## Stored procedures - A successfully executed stored procedure always returns the affected row count - of the last statement. This means any stored procedure containing statements that - return result sets, such as `SELECT` statements, must use `prepare_execute_many/5` - and similar functions. These functions will return one result for each statement - returning a result set and one for the affected row count of the last statement. + A stored procedure containing statements that return rows, such as `SELECT` + statements, must use `prepare_execute_many/5` and similar functions. This is because + stored procedures always return a final result with the number of rows affected + by the last statement. + + Take, for example, the following stored procedure: + + DELIMITER $$ + CREATE PROCEDURE stored_procedure() + BEGIN + SELECT 1; + END$$ + DELIMITER ; + + There will be one result for the `SELECT 1` statement and another result + stating that 0 rows were affected by the last statement. + + In contrast to this, a stored procedure that doesn't contain row-returning + statements can use this function. In this scenario, only the final result + with the number of rows affected by the last statment will be returned. + + Take, for example, the following stored procedure: + + DELIMITER $$ + CREATE PROCEDURE stored_procedure() + BEGIN + CREATE TABLE IF NOT EXISTS integers (x int); + INSERT INTO integers VALUES (10); + END$$ + DELIMITER ; + + Because `CREATE` and `INSERT` statements do not return rows, there will be a + single result stating that 1 row was affected by the last statement. This is + referencing the row that was inserted. ## Options @@ -559,11 +646,40 @@ defmodule MyXQL do ## Stored procedures - A successfully executed stored procedure always returns the affected row count - of the last statement. This means any stored procedure containing statements that - return result sets, such as `SELECT` statements, must use execute_many/4 and - similar functions. These functions will return one result for each statement - returning a result set and one for the affected row count of the last statement. + A stored procedure containing statements that return rows, such as `SELECT` + statements, must use `execute_many/4` and similar functions. This is because + stored procedures always return a final result with the number of rows affected + by the last statement. + + Take, for example, the following stored procedure: + + DELIMITER $$ + CREATE PROCEDURE stored_procedure() + BEGIN + SELECT 1; + END$$ + DELIMITER ; + + There will be one result for the `SELECT 1` statement and another result + stating that 0 rows were affected by the last statement. + + In contrast to this, a stored procedure that doesn't contain row-returning + statements can use this function. In this scenario, only the final result + with the number of rows affected by the last statment will be returned. + + Take, for example, the following stored procedure: + + DELIMITER $$ + CREATE PROCEDURE stored_procedure() + BEGIN + CREATE TABLE IF NOT EXISTS integers (x int); + INSERT INTO integers VALUES (10); + END$$ + DELIMITER ; + + Because `CREATE` and `INSERT` statements do not return rows, there will be a + single result stating that 1 row was affected by the last statement. This is + referencing the row that was inserted. ## Options diff --git a/lib/myxql/connection.ex b/lib/myxql/connection.ex index dadba16..a8b3c8a 100644 --- a/lib/myxql/connection.ex +++ b/lib/myxql/connection.ex @@ -273,8 +273,8 @@ defmodule MyXQL.Connection do ## Internals # This error can only be received when attempting to stream a stored procedure. - # Attemping to stream a text query with multiple-statements separated by a semi-colon - # will return a syntax error. + # Attemping to stream a text query with statements separated by semi-colons will + # return a syntax error. defp stream_result({:error, :multiple_results}, _query, _state) do raise RuntimeError, "streaming stored procedures is not supported. Use MyXQL.query_many/4 and similar functions." From 0b8552f29af532959330997e88a07c42d5b09e4c Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Wed, 16 Mar 2022 23:41:10 -0400 Subject: [PATCH 6/6] typo fix --- lib/myxql.ex | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/myxql.ex b/lib/myxql.ex index e8bc127..17f2532 100644 --- a/lib/myxql.ex +++ b/lib/myxql.ex @@ -232,7 +232,7 @@ defmodule MyXQL do In contrast to this, a stored procedure that doesn't contain row-returning statements can use this function. In this scenario, only the final result - with the number of rows affected by the last statment will be returned. + with the number of rows affected by the last statement will be returned. Take, for example, the following stored procedure: @@ -423,7 +423,7 @@ defmodule MyXQL do In contrast to this, a stored procedure that doesn't contain row-returning statements can use this function. In this scenario, only the final result - with the number of rows affected by the last statment will be returned. + with the number of rows affected by the last statement will be returned. Take, for example, the following stored procedure: @@ -544,7 +544,7 @@ defmodule MyXQL do In contrast to this, a stored procedure that doesn't contain row-returning statements can use this function. In this scenario, only the final result - with the number of rows affected by the last statment will be returned. + with the number of rows affected by the last statement will be returned. Take, for example, the following stored procedure: @@ -665,7 +665,7 @@ defmodule MyXQL do In contrast to this, a stored procedure that doesn't contain row-returning statements can use this function. In this scenario, only the final result - with the number of rows affected by the last statment will be returned. + with the number of rows affected by the last statement will be returned. Take, for example, the following stored procedure: