Filter out full-text catalog operations from history table creation#38050
Filter out full-text catalog operations from history table creation#38050
Conversation
When the migration history table is created, the model differ generates AlterDatabaseOperation with full-text catalog annotations. The SQL generator then emits CREATE FULLTEXT CATALOG too early, causing the later actual migration to fail. Filter out SqlServerAnnotationNames.FullTextCatalogs from AlterDatabaseOperation in SqlServerHistoryRepository.GetCreateCommands(), following the same approach as npgsql/efcore.pg#3713. This is a workaround for #34991. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR prevents SQL Server full-text catalog DDL from being generated during migration history table creation, avoiding failures when the actual migration later creates the same catalogs. It does this by making the history-table mini-model accessible to providers and filtering out full-text catalog annotations before SQL generation.
Changes:
- Exposes
HistoryRepository.EnsureModel()asprotected(internal API) so providers can reuse the history-table mini-model without duplicating logic. - Overrides
SqlServerHistoryRepository.GetCreateCommands()to removeSqlServerAnnotationNames.FullTextCatalogsfrom anyAlterDatabaseOperationbefore SQL generation. - Adds a SQL Server test intended to cover history-table creation when full-text catalogs are configured.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/EFCore.SqlServer.Tests/Migrations/SqlServerHistoryRepositoryTest.cs | Adds a regression test and test plumbing to configure a model with a full-text catalog. |
| src/EFCore.SqlServer/Migrations/Internal/SqlServerHistoryRepository.cs | Filters FullTextCatalogs annotations out of AlterDatabaseOperation when generating history-table create commands. |
| src/EFCore.Relational/Migrations/HistoryRepository.cs | Makes EnsureModel() protected (internal API) so provider subclasses can reuse the history-table mini-model. |
test/EFCore.SqlServer.Tests/Migrations/SqlServerHistoryRepositoryTest.cs
Show resolved
Hide resolved
- Make EnsureModel() protected virtual to fix API consistency test - Replace configureModel approach with IConventionSetPlugin injection to properly test that the full-text catalog filtering override works when conventions add annotations to the history table's mini-model Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| { | ||
| if (operation is AlterDatabaseOperation alterDatabaseOperation) | ||
| { | ||
| alterDatabaseOperation.RemoveAnnotation(SqlServerAnnotationNames.FullTextCatalogs); |
There was a problem hiding this comment.
It would be better to do this in SqlServerMigrationsSqlGenerator.RewriteOperations
There was a problem hiding this comment.
But isn't the point that we should only be filtering these operations out when creating the history repository (hence this type)? If we filter out in the migrations SQL generator we'd also stop creating the full-text catalogs as part of the regular migrations, no?
There was a problem hiding this comment.
You are right, doing it there would also be a hack. Perhaps we should just do #34991 instead
There was a problem hiding this comment.
FWIW the hack in this PR has been in place for the PG provider for quite a while... That doesn't make it great, but I'm not sure we should spend any more time on this than we have to right now...
PR #37652 added migration support for SQL Server full-text catalogs. Full-text catalogs are stored as database-level annotations (
SqlServerAnnotationNames.FullTextCatalogs) on the model, and when the migration history table is created, the model differ generates anAlterDatabaseOperationthat includes these annotations. The SQL generator then emitsCREATE FULLTEXT CATALOGprematurely — before the actual migration runs — causing the later migration (which also creates the catalog) to fail.This is the same problem that the Npgsql provider had with PostgreSQL extensions/enums, fixed in npgsql/efcore.pg#3713. The approach here mirrors that fix:
GetCreateCommands()inSqlServerHistoryRepositoryto stripFullTextCatalogsannotations fromAlterDatabaseOperationbefore SQL generationEnsureModel()protected (with[EntityFrameworkInternal]) in the baseHistoryRepositoryso the SQL Server subclass can call it — avoiding the code duplication that the Npgsql provider had to resort toThis is a workaround for the proper fix tracked in #34991.