Support int8 migration versions via new int8-versions feature

This addresses, though does not really *fix* #83, because it doesn't make refinery support timestamped migrations *by default*, but only if you opt-in to the new feature.
However, making it an optional feature neatly sidesteps the unanswered questions in the issue, and so makes the implementation easier to complete and land.

Author: mpalmer

README.md

@@ -53,15 +53,17 @@ fn main() {
 For more library examples, refer to the [`examples`](https://github.com/rust-db/refinery/tree/main/examples).
 ### Example: CLI
 
-NOTE: 
+NOTE:
 
-- Contiguous (adjacent) migration version numbers are restricted to `u32` (unsigned, 32-bit integers).
-- Non-contiguous (not adjacent) migration version numbers are restricted to `u32` (unsigned, 32-bit integers).
+- By default, migration version numbers are restricted to `i32` (signed, 32-bit integers).
+- If you enable the `int8-versions` feature, this restriction is lifted to being able to use `i64`s for your migration version numbers (yay timestamps!).
+  Bear in mind that this feature must be enabled *before* you start using refinery on a given database.
+  Migrating an existing database's `refinery_schema_history` table to use `int8` versions will break the checksums on all previously-applied migrations, which is... bad.
 
 ```bash
 export DATABASE_URL="postgres://postgres:secret@localhost:5432/your-db"
 pushd migrations
-    # Runs ./src/V1__*.rs or ./src/V1__*.sql 
+    # Runs ./src/V1__*.rs or ./src/V1__*.sql
     refinery migrate -e DATABASE_URL -p ./src -t 1
 popd
 ```

refinery/Cargo.toml

@@ -25,6 +25,7 @@ tiberius-config = ["refinery-core/tiberius", "refinery-core/tiberius-config"]
 serde = ["refinery-core/serde"]
 toml = ["refinery-core/toml"]
 enums = ["refinery-macros/enums"]
+int8-versions = ["refinery-core/int8-versions"]
 
 [dependencies]
 refinery-core = { version = "0.8.14", path = "../refinery_core" }

refinery/src/lib.rs

@@ -32,7 +32,7 @@ for more examples refer to the [examples](https://github.com/rust-db/refinery/tr
 */
 
 pub use refinery_core::config;
-pub use refinery_core::{error, load_sql_migrations, Error, Migration, Report, Runner, Target};
+pub use refinery_core::{error, load_sql_migrations, Error, Migration, Report, Runner, SchemaVersion, Target};
 #[doc(hidden)]
 pub use refinery_core::{AsyncMigrate, Migrate};
 pub use refinery_macros::embed_migrations;

refinery/tests/migrations_int8/U20240504090241__initial.rs

@@ -0,0 +1,15 @@
+use barrel::{types, Migration};
+
+use crate::Sql;
+
+pub fn migration() -> String {
+    let mut m = Migration::new();
+
+    m.create_table("persons", |t| {
+        t.add_column("id", types::primary());
+        t.add_column("name", types::varchar(255));
+        t.add_column("city", types::varchar(255));
+    });
+
+    m.make::<Sql>()
+}

refinery/tests/migrations_int8/U20240504090301__add_cars_and_motos_table.sql

@@ -0,0 +1,8 @@
+CREATE TABLE cars (
+    id int,
+    name varchar(255)
+);
+CREATE TABLE motos (
+    id int,
+    name varchar(255)
+);

refinery/tests/migrations_int8/U20240504090322__add_brand_to_cars_table.sql

@@ -0,0 +1,2 @@
+ALTER TABLE cars
+ADD brand varchar(255);

refinery/tests/migrations_int8/U20240504090343__add_year_to_motos_table.rs

@@ -0,0 +1,13 @@
+use barrel::{types, Migration};
+
+use crate::Sql;
+
+pub fn migration() -> String {
+    let mut m = Migration::new();
+
+    m.change_table("motos", |t| {
+        t.add_column("brand", types::varchar(255).nullable(true));
+    });
+
+    m.make::<Sql>()
+}

refinery/tests/postgres.rs

@@ -31,6 +31,16 @@ mod postgres {
         embed_migrations!("./tests/migrations_missing");
     }
 
+    #[cfg(feature = "int8-versions")]
+    mod int8 {
+        use refinery::embed_migrations;
+        embed_migrations!("./tests/migrations_int8");
+    }
+
+    fn db_uri() -> String {
+        std::env::var("DB_URI").unwrap_or("postgres://postgres@localhost:5432/postgres".to_string())
+    }
+
     fn get_migrations() -> Vec<Migration> {
         embed_migrations!("./tests/migrations");
 
@@ -189,6 +199,37 @@ mod postgres {
         });
     }
 
+    #[test]
+    #[cfg(feature = "int8-versions")]
+    fn applies_migration_int8() {
+        run_test(|| {
+            let mut client = Client::connect(&db_uri(), NoTls).unwrap();
+            let report = int8::migrations::runner().run(&mut client).unwrap();
+
+            let applied_migrations = report.applied_migrations();
+
+            assert_eq!(4, applied_migrations.len());
+
+            assert_eq!(20240504090241, applied_migrations[0].version());
+            assert_eq!(20240504090301, applied_migrations[1].version());
+            assert_eq!(20240504090322, applied_migrations[2].version());
+            assert_eq!(20240504090343, applied_migrations[3].version());
+
+            client
+                .execute(
+                    "INSERT INTO persons (name, city) VALUES ($1, $2)",
+                    &[&"John Legend", &"New York"],
+                )
+                .unwrap();
+            for row in &client.query("SELECT name, city FROM persons", &[]).unwrap() {
+                let name: String = row.get(0);
+                let city: String = row.get(1);
+                assert_eq!("John Legend", name);
+                assert_eq!("New York", city);
+            }
+        });
+    }
+
     #[test]
     fn applies_migration_grouped_transaction() {
         run_test(|| {
@@ -292,8 +333,15 @@ mod postgres {
             assert_eq!("initial", migrations[0].name());
             assert_eq!("add_cars_table", applied_migrations[1].name());
 
+            #[cfg(not(feature = "int8-versions"))]
             assert_eq!(2959965718684201605, applied_migrations[0].checksum());
+            #[cfg(feature = "int8-versions")]
+            assert_eq!(13938959368620441626, applied_migrations[0].checksum());
+
+            #[cfg(not(feature = "int8-versions"))]
             assert_eq!(8238603820526370208, applied_migrations[1].checksum());
+            #[cfg(feature = "int8-versions")]
+            assert_eq!(5394706226941044339, applied_migrations[1].checksum());
         });
     }
 

refinery_cli/Cargo.toml

@@ -21,6 +21,7 @@ mysql = ["refinery-core/mysql"]
 sqlite = ["refinery-core/rusqlite"]
 sqlite-bundled = ["sqlite", "refinery-core/rusqlite-bundled"]
 mssql = ["refinery-core/tiberius-config", "tokio"]
+int8-versions = ["refinery-core/int8-versions"]
 
 [dependencies]
 refinery-core = { version = "0.8.14", path = "../refinery_core", default-features = false, features = ["toml"]  }

refinery_cli/src/cli.rs

@@ -4,6 +4,8 @@ use std::path::PathBuf;
 
 use clap::{Args, Parser};
 
+use refinery_core::SchemaVersion;
+
 #[derive(Parser)]
 #[clap(version)]
 pub enum Cli {
@@ -38,7 +40,7 @@ pub struct MigrateArgs {
 
     /// Migrate to the specified target version
     #[clap(short)]
-    pub target: Option<u32>,
+    pub target: Option<SchemaVersion>,
 
     /// Set migration table name
     #[clap(long, default_value = "refinery_schema_history")]

refinery_cli/src/migrate.rs

@@ -3,7 +3,7 @@ use std::path::Path;
 use anyhow::Context;
 use refinery_core::{
     config::{Config, ConfigDbType},
-    find_migration_files, Migration, MigrationType, Runner, Target,
+    find_migration_files, Migration, MigrationType, Runner, SchemaVersion, Target,
 };
 
 use crate::cli::MigrateArgs;
@@ -30,7 +30,7 @@ fn run_migrations(
     divergent: bool,
     missing: bool,
     fake: bool,
-    target: Option<u32>,
+    target: Option<SchemaVersion>,
     env_var_opt: Option<&str>,
     path: &Path,
     table_name: &str,

refinery_core/Cargo.toml

@@ -17,6 +17,7 @@ tokio-postgres = ["dep:tokio-postgres", "tokio", "tokio/rt"]
 mysql_async = ["dep:mysql_async"]
 serde = ["dep:serde"]
 toml = ["serde", "dep:toml"]
+int8-versions = []
 
 [dependencies]
 async-trait = "0.1"

refinery_core/src/drivers/mysql_async.rs

@@ -1,4 +1,5 @@
 use crate::traits::r#async::{AsyncMigrate, AsyncQuery, AsyncTransaction};
+use crate::util::SchemaVersion;
 use crate::Migration;
 use async_trait::async_trait;
 use mysql_async::{
@@ -16,7 +17,7 @@ async fn query_applied_migrations<'a>(
     let applied = result
         .into_iter()
         .map(|row| {
-            let (version, name, applied_on, checksum): (i32, String, String, String) =
+            let (version, name, applied_on, checksum): (SchemaVersion, String, String, String) =
                 mysql_async::from_row(row);
 
             // Safe to call unwrap, as we stored it in RFC3339 format on the database

refinery_core/src/drivers/tiberius.rs

@@ -1,4 +1,5 @@
 use crate::traits::r#async::{AsyncMigrate, AsyncQuery, AsyncTransaction};
+use crate::util::SchemaVersion;
 use crate::Migration;
 
 use async_trait::async_trait;
@@ -19,7 +20,7 @@ async fn query_applied_migrations<S: AsyncRead + AsyncWrite + Unpin + Send>(
     // Unfortunately too many unwraps as `Row::get` maps to Option<T> instead of T
     while let Some(item) = rows.try_next().await? {
         if let QueryItem::Row(row) = item {
-            let version = row.get::<i32, usize>(0).unwrap();
+            let version = row.get::<SchemaVersion, usize>(0).unwrap();
             let applied_on: &str = row.get::<&str, usize>(2).unwrap();
             // Safe to call unwrap, as we stored it in RFC3339 format on the database
             let applied_on = OffsetDateTime::parse(applied_on, &Rfc3339).unwrap();

refinery_core/src/lib.rs

@@ -10,7 +10,7 @@ pub use crate::runner::{Migration, Report, Runner, Target};
 pub use crate::traits::r#async::AsyncMigrate;
 pub use crate::traits::sync::Migrate;
 pub use crate::util::{
-    find_migration_files, load_sql_migrations, parse_migration_name, MigrationType,
+    find_migration_files, load_sql_migrations, parse_migration_name, MigrationType, SchemaVersion,
 };
 
 #[cfg(feature = "rusqlite")]

refinery_core/src/runner.rs

@@ -8,7 +8,7 @@ use std::fmt;
 use std::hash::{Hash, Hasher};
 
 use crate::traits::{sync::migrate as sync_migrate, DEFAULT_MIGRATION_TABLE_NAME};
-use crate::util::parse_migration_name;
+use crate::util::{parse_migration_name, SchemaVersion};
 use crate::{AsyncMigrate, Error, Migrate};
 use std::fmt::Formatter;
 
@@ -43,9 +43,9 @@ impl fmt::Debug for Type {
 #[derive(Clone, Copy, Debug)]
 pub enum Target {
     Latest,
-    Version(u32),
+    Version(SchemaVersion),
     Fake,
-    FakeVersion(u32),
+    FakeVersion(SchemaVersion),
 }
 
 // an Enum set that represents the state of the migration: Applied on the database,
@@ -66,7 +66,7 @@ pub struct Migration {
     state: State,
     name: String,
     checksum: u64,
-    version: i32,
+    version: SchemaVersion,
     prefix: Type,
     sql: Option<String>,
     applied_on: Option<OffsetDateTime>,
@@ -105,7 +105,7 @@ impl Migration {
 
     // Create a migration from an applied migration on the database
     pub fn applied(
-        version: i32,
+        version: SchemaVersion,
         name: String,
         applied_on: OffsetDateTime,
         checksum: u64,
@@ -134,8 +134,8 @@ impl Migration {
     }
 
     /// Get the Migration version
-    pub fn version(&self) -> u32 {
-        self.version as u32
+    pub fn version(&self) -> SchemaVersion {
+        self.version
     }
 
     /// Get the Prefix

refinery_core/src/traits/async.rs

@@ -122,7 +122,14 @@ where
 {
     // Needed cause some database vendors like Mssql have a non sql standard way of checking the migrations table
     fn assert_migrations_table_query(migration_table_name: &str) -> String {
-        ASSERT_MIGRATIONS_TABLE_QUERY.replace("%MIGRATION_TABLE_NAME%", migration_table_name)
+        #[cfg(not(feature = "int8-versions"))]
+        let version_type = "int4";
+        #[cfg(feature = "int8-versions")]
+        let version_type = "int8";
+
+        ASSERT_MIGRATIONS_TABLE_QUERY
+            .replace("%MIGRATION_TABLE_NAME%", migration_table_name)
+            .replace("%VERSION_TYPE%", version_type)
     }
 
     async fn get_last_applied_migration(

refinery_core/src/traits/mod.rs

@@ -4,6 +4,7 @@ pub mod r#async;
 pub mod sync;
 
 use crate::runner::Type;
+use crate::util::SchemaVersion;
 use crate::{error::Kind, Error, Migration};
 
 // Verifies applied and to be applied migrations returning Error if:
@@ -49,10 +50,10 @@ pub(crate) fn verify_migrations(
         }
     }
 
-    let current: i32 = match applied.last() {
+    let current: SchemaVersion = match applied.last() {
         Some(last) => {
             log::info!("current version: {}", last.version());
-            last.version() as i32
+            last.version() as SchemaVersion
         }
         None => {
             log::info!("schema history table is empty, going to apply all migrations");
@@ -73,7 +74,7 @@ pub(crate) fn verify_migrations(
             if to_be_applied.contains(&migration) {
                 return Err(Error::new(Kind::RepeatedVersion(migration), None));
             } else if migration.prefix() == &Type::Versioned
-                && current >= migration.version() as i32
+                && current >= migration.version() as SchemaVersion
             {
                 if abort_missing {
                     return Err(Error::new(Kind::MissingVersion(migration), None));
@@ -105,7 +106,7 @@ pub(crate) fn insert_migration_query(migration: &Migration, migration_table_name
 
 pub(crate) const ASSERT_MIGRATIONS_TABLE_QUERY: &str =
     "CREATE TABLE IF NOT EXISTS %MIGRATION_TABLE_NAME%(
-             version INT4 PRIMARY KEY,
+             version %VERSION_TYPE% PRIMARY KEY,
              name VARCHAR(255),
              applied_on VARCHAR(255),
              checksum VARCHAR(255));";

refinery_core/src/traits/sync.rs

@@ -90,10 +90,16 @@ where
     Self: Sized,
 {
     fn assert_migrations_table(&mut self, migration_table_name: &str) -> Result<usize, Error> {
+        #[cfg(not(feature = "int8-versions"))]
+        let version_type = "int4";
+        #[cfg(feature = "int8-versions")]
+        let version_type = "int8";
+
         // Needed cause some database vendors like Mssql have a non sql standard way of checking the migrations table,
         // thou on this case it's just to be consistent with the async trait `AsyncMigrate`
         self.execute(&[ASSERT_MIGRATIONS_TABLE_QUERY
             .replace("%MIGRATION_TABLE_NAME%", migration_table_name)
+            .replace("%VERSION_TYPE%", version_type)
             .as_str()])
             .migration_err("error asserting migrations table", None)
     }

refinery_core/src/util.rs

@@ -7,6 +7,11 @@ use std::path::{Path, PathBuf};
 use std::sync::OnceLock;
 use walkdir::{DirEntry, WalkDir};
 
+#[cfg(not(feature = "int8-versions"))]
+pub type SchemaVersion = i32;
+#[cfg(feature = "int8-versions")]
+pub type SchemaVersion = i64;
+
 const STEM_RE: &'static str = r"^([U|V])(\d+(?:\.\d+)?)__(\w+)";
 
 /// Matches the stem of a migration file.
@@ -44,12 +49,12 @@ impl MigrationType {
 }
 
 /// Parse a migration filename stem into a prefix, version, and name.
-pub fn parse_migration_name(name: &str) -> Result<(Type, i32, String), Error> {
+pub fn parse_migration_name(name: &str) -> Result<(Type, SchemaVersion, String), Error> {
     let captures = file_stem_re()
         .captures(name)
         .filter(|caps| caps.len() == 4)
         .ok_or_else(|| Error::new(Kind::InvalidName, None))?;
-    let version: i32 = captures[2]
+    let version: SchemaVersion = captures[2]
         .parse()
         .map_err(|_| Error::new(Kind::InvalidVersion, None))?;