Merge pull request #1641 from Kobzol/db-error

Mark-Simulacrum · web-flow · commit 229bc651ab0e · 2023-07-16T19:15:46.000-04:00
Change how errors are stored in the DB
diff --git a/database/src/bin/postgres-to-sqlite.rs b/database/src/bin/postgres-to-sqlite.rs
@@ -191,12 +191,12 @@ impl Table for Error {
     }
 
     fn postgres_select_statement(&self, since_weeks_ago: Option<u32>) -> String {
-        let s = "select series, aid, error from ".to_string() + self.name();
+        let s = "select benchmark, aid, error from ".to_string() + self.name();
         with_filter_clause_maybe(s, ARTIFACT_JOIN_AND_WHERE, since_weeks_ago)
     }
 
     fn sqlite_insert_statement(&self) -> &'static str {
-        "insert into error (series, aid, error) VALUES (?, ?, ?)"
+        "insert into error (benchmark, aid, error) VALUES (?, ?, ?)"
     }
 
     fn sqlite_execute_insert(&self, statement: &mut rusqlite::Statement, row: tokio_postgres::Row) {
@@ -210,28 +210,6 @@ impl Table for Error {
     }
 }
 
-struct ErrorSeries;
-
-impl Table for ErrorSeries {
-    fn name(&self) -> &'static str {
-        "error_series"
-    }
-
-    fn postgres_select_statement(&self, _since_weeks_ago: Option<u32>) -> String {
-        "select id, crate from ".to_string() + self.name()
-    }
-
-    fn sqlite_insert_statement(&self) -> &'static str {
-        "insert into error_series (id, crate) VALUES (?, ?)"
-    }
-
-    fn sqlite_execute_insert(&self, statement: &mut rusqlite::Statement, row: tokio_postgres::Row) {
-        statement
-            .execute(params![row.get::<_, i32>(0), row.get::<_, &str>(1)])
-            .unwrap();
-    }
-}
-
 struct Pstat;
 
 impl Table for Pstat {
@@ -450,7 +428,6 @@ async fn main() -> anyhow::Result<()> {
         &Benchmark,
         &Collection,
         &CollectorProgress,
-        &ErrorSeries,
         &Error,
         &PstatSeries,
         &Pstat,
diff --git a/database/src/bin/sqlite-to-postgres.rs b/database/src/bin/sqlite-to-postgres.rs
@@ -218,7 +218,7 @@ struct Error;
 
 #[derive(Serialize)]
 struct ErrorRow<'a> {
-    series: i32,
+    benchmark: &'a str,
     aid: i32,
     error: Nullable<&'a str>,
 }
@@ -229,7 +229,7 @@ impl Table for Error {
     }
 
     fn sqlite_attributes() -> &'static str {
-        "series, aid, error"
+        "benchmark, aid, error"
     }
 
     fn postgres_generated_id_attribute() -> Option<&'static str> {
@@ -239,45 +239,14 @@ impl Table for Error {
     fn write_postgres_csv_row<W: Write>(writer: &mut csv::Writer<W>, row: &rusqlite::Row) {
         writer
             .serialize(ErrorRow {
-                series: row.get(0).unwrap(),
+                benchmark: row.get_ref(0).unwrap().as_str().unwrap(),
                 aid: row.get(1).unwrap(),
                 error: row.get_ref(2).unwrap().try_into().unwrap(),
             })
             .unwrap();
     }
 }
 
-struct ErrorSeries;
-
-#[derive(Serialize)]
-struct ErrorSeriesRow<'a> {
-    id: i32,
-    krate: &'a str,
-}
-
-impl Table for ErrorSeries {
-    fn name() -> &'static str {
-        "error_series"
-    }
-
-    fn sqlite_attributes() -> &'static str {
-        "id, crate"
-    }
-
-    fn postgres_generated_id_attribute() -> Option<&'static str> {
-        Some("id")
-    }
-
-    fn write_postgres_csv_row<W: Write>(writer: &mut csv::Writer<W>, row: &rusqlite::Row) {
-        writer
-            .serialize(ErrorSeriesRow {
-                id: row.get(0).unwrap(),
-                krate: row.get_ref(1).unwrap().as_str().unwrap(),
-            })
-            .unwrap();
-    }
-}
-
 struct Pstat;
 
 #[derive(Serialize)]
@@ -664,7 +633,6 @@ async fn main() -> anyhow::Result<()> {
     copy::<Benchmark>(&sqlite_tx, &postgres_tx).await;
     copy::<Collection>(&sqlite_tx, &postgres_tx).await;
     copy::<CollectorProgress>(&sqlite_tx, &postgres_tx).await;
-    copy::<ErrorSeries>(&sqlite_tx, &postgres_tx).await;
     copy::<Error>(&sqlite_tx, &postgres_tx).await;
     copy::<PstatSeries>(&sqlite_tx, &postgres_tx).await;
     copy::<Pstat>(&sqlite_tx, &postgres_tx).await;
diff --git a/database/src/pool/postgres.rs b/database/src/pool/postgres.rs
@@ -229,6 +229,22 @@ static MIGRATIONS: &[&str] = &[
         value double precision not null,
         PRIMARY KEY(series, aid, cid)
     );
+"#,
+    r#"
+    create table error_new(
+        aid integer not null references artifact(id) on delete cascade on update cascade,
+        benchmark text not null,
+        error text not null,
+        primary key(aid, benchmark)
+    );
+    insert into error_new(aid, benchmark, error)
+    select aid, crate, error
+    from error
+    join error_series es on error.series = es.id;
+
+    drop table error;
+    drop table error_series;
+    alter table error_new rename to error;
 "#,
 ];
 
@@ -368,7 +384,7 @@ impl PostgresConnection {
         PostgresConnection {
             statements: Arc::new(CachedStatements {
                 get_pstat: conn
-                     .prepare("
+                    .prepare("
                          WITH aids AS (
                              select aid, num from unnest($2::int[]) with ordinality aids(aid, num)
                          ),
@@ -432,8 +448,7 @@ impl PostgresConnection {
                     )
                     .await
                     .unwrap(),
-                get_error: conn.prepare("select crate, error from error_series
-                    inner join error on error.series = error_series.id and aid = $1").await.unwrap(),
+                get_error: conn.prepare("select benchmark, error from error where aid = $1").await.unwrap(),
                 select_self_query_series: conn.prepare("select id from self_profile_query_series where crate = $1 and profile = $2 and cache = $3 and query = $4").await.unwrap(),
                 insert_self_query_series: conn.prepare("insert into self_profile_query_series (crate, profile, cache, query) VALUES ($1, $2, $3, $4) ON CONFLICT DO NOTHING RETURNING id").await.unwrap(),
                 insert_pstat_series: conn.prepare("insert into pstat_series (crate, profile, cache, statistic) VALUES ($1, $2, $3, $4) ON CONFLICT DO NOTHING RETURNING id").await.unwrap(),
@@ -700,7 +715,7 @@ where
                 &[&(pr as i32), &include, &exclude, &runs],
             )
             .await {
-                log::error!("failed to queue_pr({}, {:?}, {:?}, {:?}): {:?}", pr, include, exclude, runs, e);
+            log::error!("failed to queue_pr({}, {:?}, {:?}, {:?}): {:?}", pr, include, exclude, runs, e);
         }
     }
     async fn pr_attach_commit(
@@ -907,13 +922,13 @@ where
             Some(aid) => aid.get::<_, i32>(0) as u32,
             None => {
                 self.conn()
-            .query_opt("insert into artifact (name, date, type) VALUES ($1, $2, $3) ON CONFLICT DO NOTHING RETURNING id", &[
-                &name,
-                &date,
-                &ty,
-            ])
-            .await
-            .unwrap();
+                    .query_opt("insert into artifact (name, date, type) VALUES ($1, $2, $3) ON CONFLICT DO NOTHING RETURNING id", &[
+                        &name,
+                        &date,
+                        &ty,
+                    ])
+                    .await
+                    .unwrap();
                 self.conn()
                     .query_one("select id from artifact where name = $1", &[&name])
                     .await
@@ -984,27 +999,10 @@ where
     }
 
     async fn record_error(&self, artifact: ArtifactIdNumber, krate: &str, error: &str) {
-        let sid = self
-            .conn()
-            .query_opt(
-                "insert into error_series (crate) VALUES ($1) ON CONFLICT DO NOTHING RETURNING id",
-                &[&krate],
-            )
-            .await
-            .unwrap();
-        let sid: i32 = match sid {
-            Some(id) => id.get(0),
-            None => self
-                .conn()
-                .query_one("select id from error_series where crate = $1", &[&krate])
-                .await
-                .unwrap()
-                .get(0),
-        };
         self.conn()
             .execute(
-                "insert into error (series, aid, error) VALUES ($1, $2, $3)",
-                &[&sid, &(artifact.0 as i32), &error],
+                "insert into error (benchmark, aid, error) VALUES ($1, $2, $3)",
+                &[&krate, &(artifact.0 as i32), &error],
             )
             .await
             .unwrap();
diff --git a/database/src/pool/sqlite.rs b/database/src/pool/sqlite.rs
@@ -339,6 +339,24 @@ static MIGRATIONS: &[Migration] = &[
         );
         "#,
     ),
+    Migration::new(
+        r#"
+        create table error_new(
+            aid integer references artifact(id) on delete cascade on update cascade,
+            benchmark text not null,
+            error text not null,
+            primary key(aid, benchmark)
+        );
+        insert into error_new(aid, benchmark, error)
+        select aid, crate, error
+        from error
+        join error_series es on error.series = es.id;
+
+        drop table error;
+        drop table error_series;
+        alter table error_new rename to error;
+    "#,
+    ),
 ];
 
 #[async_trait::async_trait]
@@ -748,22 +766,8 @@ impl Connection for SqliteConnection {
     async fn record_error(&self, artifact: ArtifactIdNumber, krate: &str, error: &str) {
         self.raw_ref()
             .execute(
-                "insert or ignore into error_series (crate) VALUES (?)",
-                params![&krate,],
-            )
-            .unwrap();
-        let sid: i32 = self
-            .raw_ref()
-            .query_row(
-                "select id from error_series where crate = ?",
-                params![&krate,],
-                |r| r.get(0),
-            )
-            .unwrap();
-        self.raw_ref()
-            .execute(
-                "insert into error (series, aid, error) VALUES (?, ?, ?)",
-                params![&sid, &artifact.0, &error],
+                "insert into error (benchmark, aid, error) VALUES (?, ?, ?)",
+                params![krate, &artifact.0, &error],
             )
             .unwrap();
     }
@@ -907,10 +911,7 @@ impl Connection for SqliteConnection {
     }
     async fn get_error(&self, aid: crate::ArtifactIdNumber) -> HashMap<String, String> {
         self.raw_ref()
-            .prepare_cached(
-                "select crate, error from error_series
-                    inner join error on error.series = error_series.id and aid = ?",
-            )
+            .prepare_cached("select benchmark, error from error where aid = ?")
             .unwrap()
             .query_map(params![&aid.0], |row| Ok((row.get(0)?, row.get(1)?)))
             .unwrap()
