From 68237ef382826e4415e8210133ab035034bb2498 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reynir=20Bj=C3=B6rnsson?= Date: Thu, 18 Nov 2021 11:56:17 +0100 Subject: [PATCH] Rewrite queries to take optional platform A number of queries were the same except one would take another string argument and add `AND platform = $N` in its WHERE clause. This commit merges them and does `AND ($N IS NULL OR platform = $N)` and the client code in Model doesn't have to check the string option. --- db/builder_db.ml | 71 ++++++++--------------------------------------- db/builder_db.mli | 17 +++--------- lib/model.ml | 20 +++---------- 3 files changed, 19 insertions(+), 89 deletions(-) diff --git a/db/builder_db.ml b/db/builder_db.ml index 10f0dab..7acb787 100644 --- a/db/builder_db.ml +++ b/db/builder_db.ml @@ -317,29 +317,14 @@ module Build = struct let get_all_failed = Caqti_request.collect - Caqti_type.(tup2 int int) + Caqti_type.(tup3 int int (option string)) (Caqti_type.tup2 Caqti_type.string t) {| SELECT job.name, b.uuid, b.start_d, b.start_ps, b.finish_d, b.finish_ps, b.result_code, b.result_msg, b.console, b.script, b.platform, b.main_binary, b.input_id, b.user, b.job FROM build b INNER JOIN job ON job.id = b.job - WHERE b.result_code <> 0 - ORDER BY start_d DESC, start_ps DESC - LIMIT $2 - OFFSET $1 - |} - - let get_all_failed_by_platform = - Caqti_request.collect - Caqti_type.(tup3 int int string) - (Caqti_type.tup2 Caqti_type.string t) - {| SELECT job.name, b.id, b.uuid, b.start_d, b.start_ps, b.finish_d, b.finish_ps, - b.result_code, b.result_msg, b.console, b.script, b.platform, - b.main_binary, b.input_id, b.user, b.job - FROM build b - INNER JOIN job ON job.id = b.job - WHERE b.result_code <> 0 AND b.platform = $3 + WHERE b.result_code <> 0 AND ($3 IS NULL OR b.platform = $3) ORDER BY start_d DESC, start_ps DESC LIMIT $2 OFFSET $1 @@ -347,45 +332,25 @@ module Build = struct let get_all_artifact_sha = Caqti_request.collect - (id `job) + Caqti_type.(tup2 (id `job) (option string)) Rep.cstruct {| SELECT DISTINCT a.sha256 FROM build_artifact a, build b - WHERE b.job = ? AND b.main_binary = a.id - ORDER BY b.start_d DESC, b.start_ps DESC - |} - - let get_all_artifact_sha_by_platform = - Caqti_request.collect - Caqti_type.(tup2 (id `job) string) - Rep.cstruct - {| SELECT DISTINCT a.sha256 - FROM build_artifact a, build b - WHERE b.job = ? AND b.platform = ? AND b.main_binary = a.id + WHERE b.job = $1 AND b.main_binary = a.id + AND ($2 IS NULL OR b.platform = $2) ORDER BY b.start_d DESC, b.start_ps DESC |} let get_failed_builds = Caqti_request.collect - (id `job) + Caqti_type.(tup2 (id `job) (option string)) t {| SELECT uuid, start_d, start_ps, finish_d, finish_ps, result_code, result_msg, console, script, platform, main_binary, input_id, user, job FROM build - WHERE job = ? AND result_code <> 0 - ORDER BY start_d DESC, start_ps DESC - |} - - let get_failed_builds_by_platform = - Caqti_request.collect - Caqti_type.(tup2 (id `job) string) - t - {| SELECT uuid, start_d, start_ps, finish_d, finish_ps, - result_code, result_msg, console, script, - platform, main_binary, input_id, user, job - FROM build - WHERE job = ? AND platform = ? AND result_code <> 0 + WHERE job = $1 AND result_code <> 0 + AND ($2 IS NULL OR platform = $2) ORDER BY start_d DESC, start_ps DESC |} @@ -411,33 +376,19 @@ module Build = struct let get_latest_successful = Caqti_request.find_opt - (id `job) + Caqti_type.(tup2 (id `job) (option string)) t {| SELECT b.uuid, b.start_d, b.start_ps, b.finish_d, b.finish_ps, b.result_code, b.result_msg, b.console, b.script, b.platform, b.main_binary, b.input_id, b.user, b.job FROM build b - WHERE b.job = ? AND b.result_code = 0 + WHERE b.job = $1 AND b.result_code = 0 + AND ($2 IS NULL OR b.platform = $2) ORDER BY b.start_d DESC, b.start_ps DESC LIMIT 1 |} - let get_latest_successful_by_platform = - Caqti_request.find_opt - Caqti_type.(tup2 (id `job) string) - t - {| SELECT - b.uuid, b.start_d, b.start_ps, b.finish_d, b.finish_ps, - b.result_code, b.result_msg, b.console, b.script, - b.platform, b.main_binary, b.input_id, b.user, b.job - FROM build b - WHERE b.job = $1 AND b.result_code = 0 AND b.platform = $2 - ORDER BY b.start_d DESC, b.start_ps DESC - LIMIT 1 - |} - - let get_previous_successful_different_output = Caqti_request.find_opt (id `build) diff --git a/db/builder_db.mli b/db/builder_db.mli index 34c46e8..b25d5a0 100644 --- a/db/builder_db.mli +++ b/db/builder_db.mli @@ -114,25 +114,16 @@ sig val get_all : ([`job] id, [`build] id * t, [ `Many | `One | `Zero ]) Caqti_request.t val get_all_failed : - (int * int, string * t, [ `Many | `One | `Zero ]) Caqti_request.t - val get_all_failed_by_platform : - (int * int * string, string * t, [ `Many | `One | `Zero ]) Caqti_request.t + (int * int * string option, string * t, [ `Many | `One | `Zero ]) Caqti_request.t val get_all_artifact_sha : - ([`job] id, Cstruct.t, [ `Many | `One | `Zero ]) Caqti_request.t - val get_all_artifact_sha_by_platform : - ([`job] id * string, Cstruct.t, [ `Many | `One | `Zero ]) Caqti_request.t + ([`job] id * string option, Cstruct.t, [ `Many | `One | `Zero ]) Caqti_request.t val get_latest_successful_with_binary : ([`job] id * string, [`build] id * t * file option, [< `Many | `One | `Zero > `One `Zero ]) Caqti_request.t val get_failed_builds : - ([`job] id, t, [ `Many | `One | `Zero ]) Caqti_request.t - val get_failed_builds_by_platform : - ([`job] id * string, t, [ `Many | `One | `Zero ]) Caqti_request.t + ([`job] id * string option, t, [ `Many | `One | `Zero ]) Caqti_request.t val get_latest_successful : - ([`job] id, t, [< `Many | `One | `Zero > `One `Zero ]) - Caqti_request.t - val get_latest_successful_by_platform : - ([`job] id * string, t, [< `Many | `One | `Zero > `One `Zero ]) + ([`job] id * string option, t, [< `Many | `One | `Zero > `One `Zero ]) Caqti_request.t val get_previous_successful_different_output : ([`build] id, t, [< `Many | `One | `Zero > `One `Zero ]) diff --git a/lib/model.ml b/lib/model.ml index 956c30b..78db10a 100644 --- a/lib/model.ml +++ b/lib/model.ml @@ -69,11 +69,7 @@ let build_exists uuid (module Db : CONN) = Option.is_some let latest_successful_build job_id platform (module Db : CONN) = - match platform with - | None -> - Db.find_opt Builder_db.Build.get_latest_successful job_id - | Some platform -> - Db.find_opt Builder_db.Build.get_latest_successful_by_platform (job_id, platform) + Db.find_opt Builder_db.Build.get_latest_successful (job_id, platform) let latest_successful_build_uuid job_id platform db = latest_successful_build job_id platform db >|= fun build -> @@ -86,9 +82,7 @@ let next_successful_build_different_output id (module Db : CONN) = Db.find_opt Builder_db.Build.get_next_successful_different_output id let failed_builds ~start ~count platform (module Db : CONN) = - match platform with - | None -> Db.collect_list Builder_db.Build.get_all_failed (start, count) - | Some p -> Db.collect_list Builder_db.Build.get_all_failed_by_platform (start, count, p) + Db.collect_list Builder_db.Build.get_all_failed (start, count, platform) let builds_with_different_input_and_same_main_binary id (module Db : CONN) = Db.collect_list Builder_db.Build.get_different_input_same_output_input_ids id >>= fun ids -> @@ -136,10 +130,7 @@ let job_and_readme job (module Db : CONN) = job_id, readme let builds_grouped_by_output job_id platform (module Db : CONN) = - (match platform with - | None -> Db.collect_list Builder_db.Build.get_all_artifact_sha job_id - | Some p -> Db.collect_list Builder_db.Build.get_all_artifact_sha_by_platform (job_id, p)) - >>= fun sha -> + Db.collect_list Builder_db.Build.get_all_artifact_sha (job_id, platform) >>= fun sha -> Lwt_list.fold_left_s (fun acc hash -> match acc with | Error _ as e -> Lwt.return e @@ -150,10 +141,7 @@ let builds_grouped_by_output job_id platform (module Db : CONN) = let builds_grouped_by_output_with_failed job_id platform ((module Db : CONN) as db) = builds_grouped_by_output job_id platform db >>= fun builds -> - (match platform with - | None -> Db.collect_list Builder_db.Build.get_failed_builds job_id - | Some p -> Db.collect_list Builder_db.Build.get_failed_builds_by_platform (job_id, p)) - >|= fun failed -> + Db.collect_list Builder_db.Build.get_failed_builds (job_id, platform) >|= fun failed -> let failed = List.map (fun b -> b, None) failed in let cmp (a, _) (b, _) = Ptime.compare b.Builder_db.Build.start a.Builder_db.Build.start in List.merge cmp builds failed