Update database interface & implement an extract_if method

Jamesbarford · Jamesbarford · commit 81814a0e850b · 2025-07-03T09:50:58.000+01:00
diff --git a/database/src/lib.rs b/database/src/lib.rs
@@ -816,6 +816,28 @@ impl fmt::Display for BenchmarkRequestStatus {
     }
 }
 
+impl<'a> tokio_postgres::types::FromSql<'a> for BenchmarkRequestStatus {
+    fn from_sql(
+        ty: &tokio_postgres::types::Type,
+        raw: &'a [u8],
+    ) -> Result<Self, Box<dyn std::error::Error + Sync + Send>> {
+        // Decode raw bytes into &str with Postgres' own text codec
+        let s: &str = <&str as tokio_postgres::types::FromSql>::from_sql(ty, raw)?;
+
+        match s {
+            "waiting_for_artifacts" => Ok(Self::WaitingForArtifacts),
+            "artifacts_ready" => Ok(Self::ArtifactsReady),
+            "in_progress" => Ok(Self::InProgress),
+            "completed" => Ok(Self::Completed),
+            other => Err(format!("unknown benchmark_request_status '{other}'").into()),
+        }
+    }
+
+    fn accepts(ty: &tokio_postgres::types::Type) -> bool {
+        <&str as tokio_postgres::types::FromSql>::accepts(ty)
+    }
+}
+
 #[derive(Debug, Clone, PartialEq)]
 pub enum BenchmarkRequestType {
     /// A Try commit
diff --git a/database/src/pool.rs b/database/src/pool.rs
@@ -189,7 +189,6 @@ pub trait Connection: Send + Sync {
     async fn get_benchmark_requests_by_status(
         &self,
         statuses: &[BenchmarkRequestStatus],
-        days: Option<i32>,
     ) -> anyhow::Result<Vec<BenchmarkRequest>>;
 
     /// Update the status of a `benchmark_request`
@@ -482,7 +481,7 @@ mod tests {
                 .await;
 
             let requests = db
-                .get_benchmark_requests_by_status(&[BenchmarkRequestStatus::ArtifactsReady], None)
+                .get_benchmark_requests_by_status(&[BenchmarkRequestStatus::ArtifactsReady])
                 .await
                 .unwrap();
 
@@ -495,90 +494,6 @@ mod tests {
         .await;
     }
 
-    #[tokio::test]
-    async fn get_benchmark_requests_by_status_and_date() {
-        run_postgres_test(|ctx| async {
-            let db = ctx.db_client();
-            let old_created_at = chrono::DateTime::from_str("2021-09-01T00:00:00.000Z").unwrap();
-            let created_at = Utc::now() - chrono::Duration::days(5);
-            let cutoff_days = 6;
-
-            // The following three are the only three that should be returned
-            // from the database query.
-            let master_benchmark_request = BenchmarkRequest::create_master(
-                "a-sha-1",
-                "parent-sha-1",
-                42,
-                created_at,
-                BenchmarkRequestStatus::ArtifactsReady,
-                "llvm",
-                "",
-            );
-            let master_benchmark_request_two = BenchmarkRequest::create_master(
-                "a-sha-2",
-                "parent-sha-1",
-                42,
-                created_at,
-                BenchmarkRequestStatus::Completed,
-                "llvm",
-                "",
-            );
-            let master_benchmark_request_three = BenchmarkRequest::create_master(
-                "a-sha-3",
-                "parent-sha-3",
-                42,
-                created_at,
-                BenchmarkRequestStatus::InProgress,
-                "llvm",
-                "",
-            );
-
-            let try_benchmark_request = BenchmarkRequest::create_try(
-                "b-sha-2",
-                "parent-sha-2",
-                32,
-                old_created_at,
-                BenchmarkRequestStatus::Completed,
-                "cranelift",
-                "",
-            );
-
-            let release_benchmark_request = BenchmarkRequest::create_release(
-                "1.8.0",
-                old_created_at,
-                BenchmarkRequestStatus::ArtifactsReady,
-                "cranelift,llvm",
-                "",
-            );
-
-            let db = db.connection().await;
-            db.insert_benchmark_request(&master_benchmark_request).await;
-            db.insert_benchmark_request(&master_benchmark_request_two)
-                .await;
-            db.insert_benchmark_request(&master_benchmark_request_three)
-                .await;
-            db.insert_benchmark_request(&try_benchmark_request).await;
-            db.insert_benchmark_request(&release_benchmark_request)
-                .await;
-
-            let requests = db
-                .get_benchmark_requests_by_status(
-                    &[
-                        BenchmarkRequestStatus::ArtifactsReady,
-                        BenchmarkRequestStatus::InProgress,
-                        BenchmarkRequestStatus::Completed,
-                    ],
-                    Some(cutoff_days),
-                )
-                .await;
-
-            assert_eq!(requests?.len(), 3);
-
-            Ok(ctx)
-        })
-        .await;
-    }
-
     #[tokio::test]
     async fn update_benchmark_request_status() {
         // Insert one item into the database, change the status and then
@@ -607,7 +522,7 @@ mod tests {
             .unwrap();
 
             let requests = db
-                .get_benchmark_requests_by_status(&[BenchmarkRequestStatus::InProgress], None)
+                .get_benchmark_requests_by_status(&[BenchmarkRequestStatus::InProgress])
                 .await
                 .unwrap();
 
diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs
@@ -13,7 +13,6 @@ use std::str::FromStr;
 use std::sync::Arc;
 use std::time::Duration;
 use tokio::sync::Mutex;
-use tokio_postgres::types::{FromSql, Type};
 use tokio_postgres::GenericClient;
 use tokio_postgres::Statement;
 
@@ -1418,11 +1417,10 @@ where
     async fn get_benchmark_requests_by_status(
         &self,
         statuses: &[BenchmarkRequestStatus],
-        days: Option<i32>,
     ) -> anyhow::Result<Vec<BenchmarkRequest>> {
         // There is a small period of time where a try commit's parent_sha
         // could be NULL, this query will filter that out.
-        let mut query = "
+        let query = "
                 SELECT
                     tag,
                     parent_sha,
@@ -1438,22 +1436,16 @@ where
                 AND (commit_type <> 'try' OR parent_sha IS NOT NULL) "
             .to_string();
 
-        // Optionally add interval
-        let rows = if let Some(days) = days {
-            // postgres expects the interval to be a `float` so we cast with;
-            // `::INT`, we can't do; `INTERVAL '$2 day'` which, while looking
-            // natural is invalid.
-            query += "AND created_at > current_date - $2::INT * INTERVAL '1 day'";
-            self.conn().query(&query, &[&statuses, &days]).await
-        } else {
-            self.conn().query(&query, &[&statuses]).await
-        }
-        .context("Failed to get benchmark requests")?;
+        let rows = self
+            .conn()
+            .query(&query, &[&statuses])
+            .await
+            .context("Failed to get benchmark requests")?;
 
         let benchmark_requests = rows
             .iter()
             .map(|row| {
-                let tag = row.get::<_, String>(0);
+                let tag = row.get::<_, &str>(0);
                 let parent_sha = row.get::<_, Option<String>>(1);
                 let pr = row.get::<_, Option<i32>>(2);
                 let commit_type = row.get::<_, String>(3);
@@ -1466,7 +1458,7 @@ where
                 match commit_type.as_str() {
                     "try" => {
                         let mut try_benchmark = BenchmarkRequest::create_try(
-                            &tag,
+                            tag,
                             &parent_sha.unwrap(),
                             pr.unwrap() as u32,
                             created_at,
@@ -1479,7 +1471,7 @@ where
                     }
                     "master" => {
                         let mut master_benchmark = BenchmarkRequest::create_master(
-                            &tag,
+                            tag,
                             &parent_sha.unwrap(),
                             pr.unwrap() as u32,
                             created_at,
@@ -1492,7 +1484,7 @@ where
                     }
                     "release" => {
                         let mut release_benchmark = BenchmarkRequest::create_release(
-                            &tag, created_at, status, &backends, &profiles,
+                            tag, created_at, status, &backends, &profiles,
                         );
                         release_benchmark.completed_at = completed_at;
                         release_benchmark
@@ -1575,28 +1567,6 @@ macro_rules! impl_to_postgresql_via_to_string {
 impl_to_postgresql_via_to_string!(BenchmarkRequestType);
 impl_to_postgresql_via_to_string!(BenchmarkRequestStatus);
 
-impl<'a> FromSql<'a> for BenchmarkRequestStatus {
-    fn from_sql(
-        ty: &Type,
-        raw: &'a [u8],
-    ) -> Result<Self, Box<dyn std::error::Error + Sync + Send>> {
-        // Decode raw bytes into &str with Postgres' own text codec
-        let s: &str = <&str as FromSql>::from_sql(ty, raw)?;
-
-        match s {
-            "waiting_for_artifacts" => Ok(Self::WaitingForArtifacts),
-            "artifacts_ready" => Ok(Self::ArtifactsReady),
-            "in_progress" => Ok(Self::InProgress),
-            "completed" => Ok(Self::Completed),
-            other => Err(format!("unknown benchmark_request_status '{other}'").into()),
-        }
-    }
-
-    fn accepts(ty: &Type) -> bool {
-        <&str as FromSql>::accepts(ty)
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::make_certificates;
diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs
@@ -1271,7 +1271,6 @@ impl Connection for SqliteConnection {
     async fn get_benchmark_requests_by_status(
         &self,
         _statuses: &[BenchmarkRequestStatus],
-        _days: Option<i32>,
     ) -> anyhow::Result<Vec<BenchmarkRequest>> {
         no_queue_implementation_abort!()
     }
diff --git a/database/src/tests/mod.rs b/database/src/tests/mod.rs
@@ -1,3 +1,5 @@
+#![allow(dead_code)]
+
 use std::future::Future;
 use tokio_postgres::config::Host;
 use tokio_postgres::Config;
@@ -23,7 +25,6 @@ pub struct TestContext {
 }
 
 impl TestContext {
-    #[allow(dead_code)]
     async fn new_postgres(db_url: &str) -> Self {
         let config: Config = db_url.parse().expect("Cannot parse connection string");
 
@@ -78,7 +79,6 @@ impl TestContext {
         }
     }
 
-    #[allow(dead_code)]
     async fn new_sqlite() -> Self {
         let pool = Pool::open(":memory:");
         Self {
@@ -87,12 +87,10 @@ impl TestContext {
         }
     }
 
-    #[allow(dead_code)]
     pub fn db_client(&self) -> &Pool {
         &self.client
     }
 
-    #[allow(dead_code)]
     async fn finish(self) {
         // Cleanup the test database
         // First, we need to stop using the database
@@ -118,7 +116,6 @@ impl TestContext {
 }
 
 /// Runs a test against an actual postgres database.
-#[allow(dead_code)]
 pub async fn run_postgres_test<F, Fut>(f: F)
 where
     F: Fn(TestContext) -> Fut,
@@ -146,7 +143,6 @@ where
 
 /// Runs a test against an actual database.
 /// Checks both Postgres and SQLite.
-#[allow(dead_code)]
 pub async fn run_db_test<F, Fut>(f: F)
 where
     F: Fn(TestContext) -> Fut + Clone,
diff --git a/site/src/job_queue.rs b/site/src/job_queue.rs

Original file line number	Diff line number	Diff line change
`@@ -1271,7 +1271,6 @@ impl Connection for SqliteConnection {`
`1271`	`1271`	`async fn get_benchmark_requests_by_status(`
`1272`	`1272`	`&self,`
`1273`	`1273`	`_statuses: &[BenchmarkRequestStatus],`
`1274`		`- _days: Option<i32>,`
`1275`	`1274`	`) -> anyhow::Result<Vec<BenchmarkRequest>> {`
`1276`	`1275`	`no_queue_implementation_abort!()`
`1277`	`1276`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+#![allow(dead_code)]`
	`2`	`+`
`1`	`3`	`use std::future::Future;`
`2`	`4`	`use tokio_postgres::config::Host;`
`3`	`5`	`use tokio_postgres::Config;`
`@@ -23,7 +25,6 @@ pub struct TestContext {`
`23`	`25`	`}`
`24`	`26`
`25`	`27`	`impl TestContext {`
`26`		`- #[allow(dead_code)]`
`27`	`28`	`async fn new_postgres(db_url: &str) -> Self {`
`28`	`29`	`let config: Config = db_url.parse().expect("Cannot parse connection string");`
`29`	`30`
`@@ -78,7 +79,6 @@ impl TestContext {`
`78`	`79`	`}`
`79`	`80`	`}`
`80`	`81`
`81`		`- #[allow(dead_code)]`
`82`	`82`	`async fn new_sqlite() -> Self {`
`83`	`83`	`let pool = Pool::open(":memory:");`
`84`	`84`	`Self {`
`@@ -87,12 +87,10 @@ impl TestContext {`
`87`	`87`	`}`
`88`	`88`	`}`
`89`	`89`
`90`		`- #[allow(dead_code)]`
`91`	`90`	`pub fn db_client(&self) -> &Pool {`
`92`	`91`	`&self.client`
`93`	`92`	`}`
`94`	`93`
`95`		`- #[allow(dead_code)]`
`96`	`94`	`async fn finish(self) {`
`97`	`95`	`// Cleanup the test database`
`98`	`96`	`// First, we need to stop using the database`
`@@ -118,7 +116,6 @@ impl TestContext {`
`118`	`116`	`}`
`119`	`117`
`120`	`118`	`/// Runs a test against an actual postgres database.`
`121`		`-#[allow(dead_code)]`
`122`	`119`	`pub async fn run_postgres_test<F, Fut>(f: F)`
`123`	`120`	`where`
`124`	`121`	`F: Fn(TestContext) -> Fut,`
`@@ -146,7 +143,6 @@ where`
`146`	`143`
`147`	`144`	`/// Runs a test against an actual database.`
`148`	`145`	`/// Checks both Postgres and SQLite.`
`149`		`-#[allow(dead_code)]`
`150`	`146`	`pub async fn run_db_test<F, Fut>(f: F)`
`151`	`147`	`where`
`152`	`148`	`F: Fn(TestContext) -> Fut + Clone,`