diff --git a/changelog.d/unreleased/2607.fixed.md b/changelog.d/unreleased/2607.fixed.md new file mode 100644 index 0000000000..eaddb93b79 --- /dev/null +++ b/changelog.d/unreleased/2607.fixed.md @@ -0,0 +1,16 @@ +--- +category: fixed +issues: + - 2607 +affected: + - src/CodeIndex/Database/DbContext.cs + - tests/CodeIndex.Tests/DatabaseTests.cs +--- + +## English + +- **Schema read migrations now respect existing SQLite transactions (#2607)** — `TryMigrateForRead` no longer starts a nested transaction when it is invoked inside an already-active SQLite transaction, avoiding intermittent `cannot start a transaction within a transaction` failures in concurrent freshness snapshot reads. + +## 日本語 + +- **読み取り用 schema migration が既存の SQLite transaction を尊重するようになりました (#2607)** — `TryMigrateForRead` は既存の SQLite transaction 内で呼ばれた場合に nested transaction を開始しなくなり、concurrent freshness snapshot 読み取りで断続的に発生していた `cannot start a transaction within a transaction` 失敗を回避します。 diff --git a/src/CodeIndex/Database/DbContext.cs b/src/CodeIndex/Database/DbContext.cs index 1b8d3b7e61..fcf3600bd8 100644 --- a/src/CodeIndex/Database/DbContext.cs +++ b/src/CodeIndex/Database/DbContext.cs @@ -1686,11 +1686,23 @@ public void TryMigrateForRead() try { EnsureForeignKeysEnabled(); - SqliteTransaction transaction; + SqliteTransaction? transaction; try { transaction = _connection.BeginTransaction(deferred: false); } + catch (SqliteException ex) when (IsNestedTransactionError(ex)) + { + if (RunReadMigrationSteps()) + EnsureForeignKeysEnabled(); + return; + } + catch (InvalidOperationException ex) when (IsNestedTransactionError(ex)) + { + if (RunReadMigrationSteps()) + EnsureForeignKeysEnabled(); + return; + } catch (SqliteException ex) when (IsReadOnlyOpenError(ex)) { RecordMigrationFailure("BEGIN IMMEDIATE schema migration", ex); @@ -1699,43 +1711,13 @@ public void TryMigrateForRead() using (transaction) { - _activeMigrationTransaction = transaction; - - try - { - foreach (var (description, action) in BuildReadMigrationSteps()) - { - try - { - action(); - } - catch (SqliteException ex) - { - RecordMigrationFailure(description, ex); - - // Read-only DB / filesystem / sandbox — stop further steps and degrade. - // Catches SQLITE_READONLY (8), SQLITE_IOERR (10), and SQLITE_CANTOPEN (14): - // some restricted environments report CANTOPEN when SQLite tries to create - // -journal side files for the DDL. DbReader.LoadColumns() / table-detection - // will drive the degraded read path; later read queries that hit a still- - // missing column will now have a single clear preceding diagnostic to refer to. - // 読み取り専用 DB・FS・サンドボックスでの DDL 失敗は縮退扱いで打ち切る。 - if (IsReadOnlyOpenError(ex)) return; - - // Other SQLite errors (e.g. corruption, full disk) are not opportunistic- - // migration concerns — preserve the existing surface-the-exception behavior. - // それ以外の SQLite エラーは従来通り上位に伝播させる。 - throw; - } - } - + _activeMigrationTransaction = transaction; + if (!RunReadMigrationSteps()) + return; transaction.Commit(); } - finally - { - _activeMigrationTransaction = null; - } - } + + _activeMigrationTransaction = null; EnsureForeignKeysEnabled(); } @@ -1748,6 +1730,51 @@ public void TryMigrateForRead() } } + private bool RunReadMigrationSteps() + { + try + { + foreach (var (description, action) in BuildReadMigrationSteps()) + { + try + { + action(); + } + catch (SqliteException ex) + { + RecordMigrationFailure(description, ex); + + // Read-only DB / filesystem / sandbox — stop further steps and degrade. + // Catches SQLITE_READONLY (8), SQLITE_IOERR (10), and SQLITE_CANTOPEN (14): + // some restricted environments report CANTOPEN when SQLite tries to create + // -journal side files for the DDL. DbReader.LoadColumns() / table-detection + // will drive the degraded read path; later read queries that hit a still- + // missing column will now have a single clear preceding diagnostic to refer to. + // 読み取り専用 DB・FS・サンドボックスでの DDL 失敗は縮退扱いで打ち切る。 + if (IsReadOnlyOpenError(ex)) return false; + + // Other SQLite errors (e.g. corruption, full disk) are not opportunistic- + // migration concerns — preserve the existing surface-the-exception behavior. + // それ以外の SQLite エラーは従来通り上位に伝播させる。 + throw; + } + } + + return true; + } + finally + { + _activeMigrationTransaction = null; + } + } + + private static bool IsNestedTransactionError(SqliteException exception) => + exception.SqliteErrorCode == 1 && + exception.Message.Contains("cannot start a transaction within a transaction", StringComparison.OrdinalIgnoreCase); + + private static bool IsNestedTransactionError(InvalidOperationException exception) => + exception.Message.Contains("does not support nested transactions", StringComparison.OrdinalIgnoreCase); + private void RecordMigrationFailure(string description, SqliteException exception) { var failure = new DbMigrationFailure( diff --git a/tests/CodeIndex.Tests/DatabaseTests.cs b/tests/CodeIndex.Tests/DatabaseTests.cs index fccbb6dcda..2cda50b230 100644 --- a/tests/CodeIndex.Tests/DatabaseTests.cs +++ b/tests/CodeIndex.Tests/DatabaseTests.cs @@ -486,6 +486,78 @@ public void Constructor_WritableOpenRejectsNewerUserVersion() } } + [Fact] + public void TryMigrateForRead_InsideExistingTransaction_DoesNotStartNestedTransaction() + { + var dbPath = Path.Combine(Path.GetTempPath(), $"codeindex_nested_migration_test_{Guid.NewGuid():N}.db"); + try + { + using (var connection = new SqliteConnection(new SqliteConnectionStringBuilder { DataSource = dbPath }.ConnectionString)) + { + connection.Open(); + using var cmd = connection.CreateCommand(); + cmd.CommandText = """ + CREATE TABLE files ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + path TEXT NOT NULL UNIQUE + ); + CREATE TABLE symbols ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + file_id INTEGER NOT NULL REFERENCES files(id) ON DELETE CASCADE, + kind TEXT, + name TEXT, + line INTEGER + ); + CREATE TABLE symbol_references ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + file_id INTEGER NOT NULL REFERENCES files(id) ON DELETE CASCADE, + symbol_name TEXT, + reference_kind TEXT, + line INTEGER, + column_number INTEGER, + context TEXT, + container_kind TEXT, + container_name TEXT + ); + """; + cmd.ExecuteNonQuery(); + } + + using var db = new DbContext(dbPath); + using var transaction = db.Connection.BeginTransaction(deferred: true); + + db.TryMigrateForRead(); + + using var check = db.Connection.CreateCommand(); + check.Transaction = transaction; + check.CommandText = "SELECT COUNT(*) FROM pragma_table_info('symbols') WHERE name = 'signature'"; + Assert.Equal(1L, (long)check.ExecuteScalar()!); + + transaction.Rollback(); + } + finally + { + SqliteConnection.ClearAllPools(); + if (File.Exists(dbPath)) + { + try + { + File.Delete(dbPath); + } + catch (IOException) when (OperatingSystem.IsWindows()) + { + SqliteConnection.ClearAllPools(); + File.Delete(dbPath); + } + catch (UnauthorizedAccessException) when (OperatingSystem.IsWindows()) + { + SqliteConnection.ClearAllPools(); + File.Delete(dbPath); + } + } + } + } + [Fact] public void TryMigrateForRead_EnforcesForeignKeysAfterAddingReferenceLineColumn() {