Fix grate always create Version record when no script to run.

src/grate.core/Infrastructure/ISyntax.cs

@@ -27,4 +27,5 @@ public interface ISyntax
     string Quote(string text);
     string PrimaryKeyColumn(string columnName);
     string PrimaryKeyConstraint(string tableName, string column);
+    string ResetIdentity(string schemaName, string tableName, long value);
 }

src/grate.core/Migration/AnsiSqlDatabase.cs

@@ -427,19 +427,19 @@ protected virtual string ExistsSql(string tableSchema, string fullTableName, str
 FROM {VersionTable}
 ORDER BY id DESC", 1)}
 ";
-
+    public const string NotVersioning = "0.0.0.0";
     public async Task<string> GetCurrentVersion()
     {
         try
         {
             var sql = CurrentVersionSql;
             var res = await ExecuteScalarAsync<string>(ActiveConnection, sql);
-            return res ?? "0.0.0.0";
+            return res ?? NotVersioning;
         }
         catch (Exception ex)
         {
             Logger.LogDebug(ex, "An error occurred getting the current database version, new database + --dryrun?");
-            return "0.0.0.0";
+            return NotVersioning;
         }
     }
 
@@ -480,6 +480,21 @@ public virtual async Task ChangeVersionStatus(string status, long versionId)
 
         s.Complete();
     }
+
+    public virtual async Task DeleteVersionRecord(long versionId)
+    {
+        var deleteSql = Parameterize($@"
+            DELETE FROM {VersionTable}
+            WHERE id = @versionId");
+
+        using var s = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled);
+        await ExecuteAsync(Connection, deleteSql, new { versionId });
+
+        // reset the identity column of the table to the previous value
+        await ExecuteAsync(Connection, _syntax.ResetIdentity(SchemaName, VersionTableName, versionId - 1));
+
+        s.Complete();
+    }
 
     public void Rollback()
     {

src/grate.core/Migration/GrateMigrator.cs

@@ -118,7 +118,7 @@ public async Task Migrate()
         Separator('=');
         _logger.LogInformation("Migration Scripts");
         Separator('=');
-
+        bool anySqlRun = false;
         try
         {
             // Start the transaction, if configured
@@ -151,14 +151,14 @@ public async Task Migrate()
                 {
                     if (processingFolderInDefaultTransaction)
                     {
-                        await LogAndProcess(config.SqlFilesDirectory, folder!, changeDropFolder, versionId, folder!.ConnectionType, folder.TransactionHandling);
+                        anySqlRun |= await LogAndProcess(config.SqlFilesDirectory, folder!, changeDropFolder, versionId, folder!.ConnectionType, folder.TransactionHandling);
                     }
                     else
                     {
                         using var s = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled);
                         using (await dbMigrator.OpenNewActiveConnection())
                         {
-                            await LogAndProcess(config.SqlFilesDirectory, folder!, changeDropFolder, versionId, folder!.ConnectionType, folder.TransactionHandling);
+                            anySqlRun |= await LogAndProcess(config.SqlFilesDirectory, folder!, changeDropFolder, versionId, folder!.ConnectionType, folder.TransactionHandling);
                         }
                         s.Complete();
                     }
@@ -194,8 +194,16 @@ public async Task Migrate()
 
         if (!config.DryRun)
         {
-            //If we get here this means no exceptions are thrown above, so we can conclude the migration was successfull!
-            await DbMigrator.Database.ChangeVersionStatus(MigrationStatus.Finished, versionId);
+            //If we get here this means no exceptions are thrown above, so we can conclude the migration was successful!
+            if (anySqlRun)
+            {
+                await DbMigrator.Database.ChangeVersionStatus(MigrationStatus.Finished, versionId);
+            }
+            else
+            {
+                // as we have an issue with the versioning table, we need to delete the version record
+                await DbMigrator.Database.DeleteVersionRecord(versionId);
+            }
         }
 
         _logger.LogInformation(
@@ -303,20 +311,21 @@ private static async Task RestoreDatabaseFromPath(string backupPath, IDbMigrator
 
     private static DirectoryInfo Wrap(DirectoryInfo root, string subFolder) => new(Path.Combine(root.ToString(), subFolder));
 
-    private async Task LogAndProcess(DirectoryInfo root, MigrationsFolder folder, string changeDropFolder, long versionId, ConnectionType connectionType, TransactionHandling transactionHandling)
+    private async ValueTask<bool> LogAndProcess(DirectoryInfo root, MigrationsFolder folder, string changeDropFolder, long versionId,
+        ConnectionType connectionType, TransactionHandling transactionHandling)
     {
         var path = Wrap(root, folder.Path);
 
         if (!path.Exists)
         {
             _logger.LogInformation("Skipping '{FolderName}', {Path} does not exist.", folder.Name, folder.Path);
-            return;
+            return false;
         }
 
         if (!path.EnumerateFileSystemInfos().Any()) // Ensure we check for subdirectories as well as files
         {
             _logger.LogInformation("Skipping '{FolderName}', {Path} is empty.", folder.Name, folder.Path);
-            return;
+            return false;
         }
 
         Separator(' ');
@@ -335,12 +344,13 @@ private async Task LogAndProcess(DirectoryInfo root, MigrationsFolder folder, st
             msg);
 
         Separator('-');
-        await Process(root, folder, changeDropFolder, versionId, connectionType, transactionHandling);
+        var result = await Process(root, folder, changeDropFolder, versionId, connectionType, transactionHandling);
         Separator('-');
         Separator(' ');
+        return result;
     }
 
-    private async Task Process(DirectoryInfo root, MigrationsFolder folder, string changeDropFolder, long versionId,
+    private async ValueTask<bool> Process(DirectoryInfo root, MigrationsFolder folder, string changeDropFolder, long versionId,
         ConnectionType connectionType, TransactionHandling transactionHandling)
     {
         var path = Wrap(root, folder.Path);
@@ -382,10 +392,11 @@ private async Task Process(DirectoryInfo root, MigrationsFolder folder, string c
         {
             _logger.LogInformation(" No sql run, either an empty folder, or all files run against destination previously.");
         }
+        return anySqlRun;
 
     }
 
-    private async Task<bool> ProcessWithoutLogging(DirectoryInfo root, MigrationsFolder folder, string changeDropFolder,
+    private async ValueTask<bool> ProcessWithoutLogging(DirectoryInfo root, MigrationsFolder folder, string changeDropFolder,
         ConnectionType connectionType, TransactionHandling transactionHandling)
     {
         var path = Wrap(root, folder.Path);

src/grate.core/Migration/IDatabase.cs

@@ -45,6 +45,7 @@ Task InsertScriptRun(string scriptName, string? sql, string hash, bool runOnce,
     Task InsertScriptRunError(string scriptName, string? sql, string errorSql, string errorMessage, long versionId);
     Task<bool> VersionTableExists();
     Task ChangeVersionStatus(string status, long versionId);
+    Task DeleteVersionRecord(long versionId);
     void SetDefaultConnectionActive();
     Task<IDisposable> OpenNewActiveConnection();
     Task OpenActiveConnection();

src/grate.mariadb/Infrastructure/MariaDbSyntax.cs

@@ -31,4 +31,7 @@ public string StatementSeparatorRegex
     public string Quote(string text) => $"`{text}`";
     public string PrimaryKeyConstraint(string tableName, string column) => $",\nCONSTRAINT PK_{tableName}_{column} PRIMARY KEY ({column})";
     public string LimitN(string sql, int n) => sql + "\nLIMIT 1";
+
+    // any idea to reset identity without using value is welcome.
+    public string ResetIdentity(string schemaName, string tableName, long value) => @$"ALTER TABLE {TableWithSchema(schemaName, tableName)} AUTO_INCREMENT = {value}";
 }

src/grate.oracle/Infrastructure/OracleSyntax.cs

@@ -22,7 +22,7 @@ public string StatementSeparatorRegex
     public string TextType => "CLOB";
     public string BigintType => "NUMBER(19)";
     public string BooleanType => "CHAR(1)";
-    public string PrimaryKeyColumn(string columnName) => $"{columnName} NUMBER(19) PRIMARY KEY";
+    public string PrimaryKeyColumn(string columnName) => $"{columnName} NUMBER(19) GENERATED ALWAYS AS IDENTITY NOT NULL PRIMARY KEY ";
 
     public string CreateSchema(string schemaName) => throw new NotImplementedException("Create schema is not implemented for Oracle DB");
 
@@ -46,12 +46,11 @@ FOR ln_cur IN (SELECT sid, serial# FROM v$session WHERE username = usr)
     EXECUTE IMMEDIATE 'DROP USER {databaseName} CASCADE';
 end;
 ";
-
-
     public string TableWithSchema(string schemaName, string tableName) => $"{schemaName}_{tableName}";
     public string ReturnId => "RETURNING id;";
     public string TimestampType => "timestamp";
     public string Quote(string text) => $"\"{text}\"";
     public string PrimaryKeyConstraint(string tableName, string column) => "";
     public string LimitN(string sql, int n) => sql + $"\nLIMIT {n}";
+    public string ResetIdentity(string schemaName, string tableName, long _) => $"ALTER TABLE {TableWithSchema(schemaName, tableName)} MODIFY id NUMBER(19) GENERATED BY DEFAULT ON NULL AS IDENTITY (START WITH LIMIT VALUE)";
 }

src/grate.oracle/Migration/OracleDatabase.cs

@@ -53,40 +53,40 @@ SELECT version
 WHERE  version_row_number <= 1
 ";
 
-    protected override async Task CreateScriptsRunTable()
-    {
-        if (!await ScriptsRunTableExists())
-        {
-            await base.CreateScriptsRunTable();
-            await CreateIdSequence(ScriptsRunTable);
-            await CreateIdInsertTrigger(ScriptsRunTable);
-        }
-    }
-
-    protected override async Task CreateScriptsRunErrorsTable()
-    {
-        if (!await ScriptsRunErrorsTableExists())
-        {
-            await base.CreateScriptsRunErrorsTable();
-            await CreateIdSequence(ScriptsRunErrorsTable);
-            await CreateIdInsertTrigger(ScriptsRunErrorsTable);
-        }
-    }
+    // protected override async Task CreateScriptsRunTable()
+    // {
+    //     if (!await ScriptsRunTableExists())
+    //     {
+    //         await base.CreateScriptsRunTable();
+    //         await CreateIdSequence(ScriptsRunTable);
+    //         await CreateIdInsertTrigger(ScriptsRunTable);
+    //     }
+    // }
+
+    // protected override async Task CreateScriptsRunErrorsTable()
+    // {
+    //     if (!await ScriptsRunErrorsTableExists())
+    //     {
+    //         await base.CreateScriptsRunErrorsTable();
+    //         await CreateIdSequence(ScriptsRunErrorsTable);
+    //         await CreateIdInsertTrigger(ScriptsRunErrorsTable);
+    //     }
+    // }
 
     public override Task RestoreDatabase(string backupPath)
     {
         throw new System.NotImplementedException("Restoring a database from file is not currently supported for Oracle.");
     }
 
-    protected override async Task CreateVersionTable()
-    {
-        if (!await VersionTableExists())
-        {
-            await base.CreateVersionTable();
-            await CreateIdSequence(VersionTable);
-            await CreateIdInsertTrigger(VersionTable);
-        }
-    }
+    // protected override async Task CreateVersionTable()
+    // {
+    //     if (!await VersionTableExists())
+    //     {
+    //         await base.CreateVersionTable();
+    //         await CreateIdSequence(VersionTable);
+    //         await CreateIdInsertTrigger(VersionTable);
+    //     }
+    // }
 
     protected override string Parameterize(string sql) => sql.Replace("@", ":");
     protected override object Bool(bool source) => source ? '1' : '0';
@@ -130,23 +130,24 @@ public override string DatabaseName
         }
     }
 
-    public override async Task ChangeVersionStatus(string status, long versionId)
-    {
-        var sql = (string)$@"
-            UPDATE {VersionTable}
-            SET status = :status
-            WHERE id = :versionId";
-
-        var parameters = new
-        {
-            status,
-            versionId,
-        };
-
-        await Connection.ExecuteAsync(
-            sql,
-            parameters);
-    }
+    // since the sql is parameterized, we no longer need to override this method anymore.
+    // public override async Task ChangeVersionStatus(string status, long versionId)
+    // {
+    //     var sql = (string)$@"
+    //         UPDATE {VersionTable}
+    //         SET status = :status
+    //         WHERE id = :versionId";
+
+    //     var parameters = new
+    //     {
+    //         status,
+    //         versionId,
+    //     };
+
+    //     await Connection.ExecuteAsync(
+    //         sql,
+    //         parameters);
+    // }
 
     private static IDictionary<string, string?> Tokenize(string? connectionString)
     {
@@ -158,21 +159,21 @@ await Connection.ExecuteAsync(
     private static string? GetValue(IDictionary<string, string?> dictionary, string key) =>
         dictionary.TryGetValue(key, out string? value) ? value : null;
 
-    private async Task CreateIdSequence(string table)
-    {
-        var sql = $"CREATE SEQUENCE {table}_seq";
-        await ExecuteNonQuery(ActiveConnection, sql, Config?.CommandTimeout);
-    }
-
-    private async Task CreateIdInsertTrigger(string table)
-    {
-        var sql = $@"
-CREATE OR REPLACE TRIGGER {table}_ins
-BEFORE INSERT ON {table}
-FOR EACH ROW
-BEGIN
-  SELECT {table}_seq.nextval INTO :new.id FROM dual;
-END;";
-        await ExecuteNonQuery(ActiveConnection, sql, Config?.CommandTimeout);
-    }
+    //     private async Task CreateIdSequence(string table)
+    //     {
+    //         var sql = $"CREATE SEQUENCE {table}_seq";
+    //         await ExecuteNonQuery(ActiveConnection, sql, Config?.CommandTimeout);
+    //     }
+
+    //     private async Task CreateIdInsertTrigger(string table)
+    //     {
+    //         var sql = $@"
+    // CREATE OR REPLACE TRIGGER {table}_ins
+    // BEFORE INSERT ON {table}
+    // FOR EACH ROW
+    // BEGIN
+    //   SELECT {table}_seq.nextval INTO :new.id FROM dual;
+    // END;";
+    //         await ExecuteNonQuery(ActiveConnection, sql, Config?.CommandTimeout);
+    //     }
 }

src/grate.postgresql/Infrastructure/PostgreSqlSyntax.cs

@@ -35,4 +35,5 @@ public string StatementSeparatorRegex
     public string Quote(string text) => $"\"{text}\"";
     public string PrimaryKeyConstraint(string tableName, string column) => $",\nCONSTRAINT PK_{tableName}_{column} PRIMARY KEY ({column})";
     public string LimitN(string sql, int n) => sql + $"\nLIMIT {n}";
+    public string ResetIdentity(string schemaName, string tableName, long _) => @$"SELECT setval(pg_get_serial_sequence('{TableWithSchema(schemaName, tableName)}', 'id'), coalesce(MAX(id), 1)) from {TableWithSchema(schemaName, tableName)};";
 }

src/grate.sqlite/Infrastructure/SqliteSyntax.cs

@@ -21,7 +21,7 @@ public string StatementSeparatorRegex
     public string TextType => "ntext";
     public string BigintType => "BIGINT";
     public string BooleanType => "bit";
-    public string PrimaryKeyColumn(string columnName) => $"{columnName} INTEGER PRIMARY KEY NOT NULL";
+    public string PrimaryKeyColumn(string columnName) => $"{columnName} INTEGER PRIMARY KEY AUTOINCREMENT";
     public string CreateSchema(string schemaName) => @$"CREATE SCHEMA ""{schemaName}"";";
 
     // The "Create database" is a no-op with Sqlite, so we just provide a dummy SQL that just selects current DB
@@ -36,4 +36,7 @@ public string StatementSeparatorRegex
     public string Quote(string text) => $"\"{text}\"";
     public string PrimaryKeyConstraint(string tableName, string column) => "";
     public string LimitN(string sql, int n) => sql + $"\nLIMIT {n}";
+    public string ResetIdentity(string schemaName, string tableName, long _) => @$"UPDATE `sqlite_sequence`
+                                                                                    SET `seq` = (SELECT MAX(`id`) FROM '{TableWithSchema(schemaName, tableName)}')
+                                                                                    WHERE `name` = '{TableWithSchema(schemaName, tableName)}';";
 }

src/grate.sqlserver/Infrastructure/SqlServerSyntax.cs

@@ -36,4 +36,5 @@ DROP DATABASE [{databaseName}]
     public string Quote(string text) => $"\"{text}\"";
     public string PrimaryKeyConstraint(string tableName, string column) => $",\nCONSTRAINT PK_{tableName}_{column} PRIMARY KEY CLUSTERED ({column})";
     public string LimitN(string sql, int n) => $"TOP {n}\n" + sql;
+    public string ResetIdentity(string schemaName, string tableName, long _) => @$"DECLARE @max INT SELECT @max=ISNULL(max([id]),0) from [{schemaName}].[{tableName}]; DBCC CHECKIDENT ('[{schemaName}].[{tableName}]', RESEED, @max );";
 }

unittests/MariaDB/Running_MigrationScripts/Versioning_The_Database.cs

@@ -1,4 +1,5 @@
-using MariaDB.TestInfrastructure;
+using grate.Configuration;
+using MariaDB.TestInfrastructure;
 using TestCommon.TestInfrastructure;
 
 namespace MariaDB.Running_MigrationScripts;

unittests/Oracle/Running_MigrationScripts/Versioning_The_Database.cs

@@ -0,0 +1,17 @@
+using TestCommon.TestInfrastructure;
+
+namespace Oracle.Running_MigrationScripts;
+
+[Collection(nameof(OracleTestContainer))]
+// ReSharper disable once InconsistentNaming
+// ReSharper disable once UnusedType.Global
+public class Versioning_The_Database(IGrateTestContext testContext, ITestOutputHelper testOutput)
+    : TestCommon.Generic.Running_MigrationScripts.Versioning_The_Database(testContext, testOutput)
+{
+    [Fact(Skip = "Skip due to Oracle doesn't support dynamic database creation in runtime")]
+    public override Task Does_not_create_versions_when_no_scripts_exist()
+    {
+        return Task.CompletedTask;
+    }
+}
+

unittests/PostgreSQL/Running_MigrationScripts/Versioning_The_Database.cs

@@ -1,5 +1,4 @@
-using TestCommon.TestInfrastructure;
-
+using TestCommon.TestInfrastructure;
 namespace PostgreSQL.Running_MigrationScripts;
 
 [Collection(nameof(PostgreSqlTestContainer))]