Skip to content

Commit

Permalink
Skip non-row result sets
Browse files Browse the repository at this point in the history
This make queries that return result sets like "x rows affected" get
skipped instead of being returned as an empty result set.

At some point we might want to return actual result set objects with
more info like return codes and output params, but that would require
a substantial breaking change to the API anyway.
  • Loading branch information
brendanlong committed May 5, 2020
1 parent 1402d63 commit 3bda799
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 20 deletions.
36 changes: 22 additions & 14 deletions src/client.ml
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,29 @@ let execute' ?params ~query ~formatted_query ({ month_offset; _ } as t) ~f =
Dblib.sqlexec conn formatted_query;
Iter.from_fun (fun () ->
if Dblib.results conn
then (
let colnames =
Dblib.numcols conn
|> List.range 0
|> List.map ~f:(fun i -> Dblib.colname conn (i + 1))
in
Iter.from_fun (fun () ->
try
let row = Dblib.nextrow conn in
let row = Row.create_exn ~month_offset ~colnames row in
Some row
with
| Caml.Not_found -> None)
|> Option.some)
then
if Dblib.has_rows conn
then (
let colnames =
Dblib.numcols conn
|> List.range 0
|> List.map ~f:(fun i -> Dblib.colname conn (i + 1))
in
Iter.from_fun (fun () ->
try
let row = Dblib.nextrow conn in
let row = Row.create_exn ~month_offset ~colnames row in
Some row
with
| Caml.Not_found -> None)
|> Option.some
|> Option.some)
else
(* We return Some None here so Iter.from_fun will keep repeating, and then filter out the
None result sets later *)
Some None
else None)
|> IterLabels.filter_map ~f:Fn.id
|> f)
;;

Expand Down
28 changes: 22 additions & 6 deletions test/test_mssql.ml
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,17 @@ let test_multiple_queries_in_execute_multi_result () =
>>| ae_sexp [%sexp_of: int option list list] [ [ Some 1 ]; [ Some 2 ] ]
;;

let test_not_result_queries_don't_count () =
Mssql.Test.with_conn (fun db ->
(* This query has 3 expressions but only the SELECT should actually return a result set *)
Mssql.execute_single
db
"CREATE TABLE #test (id int); INSERT INTO #test (id) VALUES (1); SELECT * FROM \
#test"
>>| Option.map ~f:(fun row -> Mssql.Row.int_exn row "id"))
>>| ae_sexp [%sexp_of: int option] (Some 1)
;;

let test_execute_unit () =
Mssql.Test.with_conn (fun db ->
[ "SET XACT_ABORT ON"
Expand Down Expand Up @@ -553,18 +564,22 @@ let test_exception_with_multiple_results () =
Mssql.execute
db
{|
CREATE TABLE #test (id INT PRIMARY KEY);
INSERT INTO #test (id) VALUES (1);
INSERT INTO #test (id) VALUES (1); -- primary key violation
SELECT * FROM #test;
|})
CREATE TABLE #test (id INT PRIMARY KEY);
INSERT INTO #test (id) VALUES (1);
INSERT INTO #test (id) VALUES (1); -- primary key violation
SELECT * FROM #test;
|})
>>| (function
| Error exn ->
if Exn.to_string_mach exn
|> String.is_substring ~substring:"PRIMARY KEY"
|> not
then raise exn
| Ok _ -> assert false)
| Ok res ->
failwithf
!"Expected an error but got results: %{sexp:Mssql.Row.t list}"
res
())
>>= fun () ->
(* if our cleanup code works right, this won't throw an exception *)
Mssql.execute db "SELECT 1" |> Deferred.ignore_m)
Expand Down Expand Up @@ -603,6 +618,7 @@ let () =
; "multiple queries in execute", test_multiple_queries_in_execute
; ( "multiple queries in execute_multi_result"
, test_multiple_queries_in_execute_multi_result )
; "test_not_result_queries_don't_count", test_not_result_queries_don't_count
; "execute_unit", test_execute_unit
; "execute_unit fail", test_execute_unit_fail
; "execute_single", test_execute_single
Expand Down

0 comments on commit 3bda799

Please sign in to comment.