API review follow-ups: rename full-text/vector builder methods, remove IndexBuilder extensions, collapse TVF overloads#38027
Open
roji wants to merge 1 commit intodotnet:mainfrom
Open
API review follow-ups: rename full-text/vector builder methods, remove IndexBuilder extensions, collapse TVF overloads#38027roji wants to merge 1 commit intodotnet:mainfrom
roji wants to merge 1 commit intodotnet:mainfrom
Conversation
…e IndexBuilder extensions, collapse TVF overloads Implements the @roji-tagged items from dotnet#37993: - Move FullTextSearchResult to Microsoft.EntityFrameworkCore.Query namespace - SqlServerFullTextIndexBuilder: HasKeyIndex→UseKeyIndex, OnCatalog→UseCatalog, WithChangeTracking→HasChangeTracking, HasLanguage→UseLanguage - SqlServerVectorIndexBuilder: UseMetric→HasMetric, UseType→HasType - Remove HasFullTextKeyIndex/Catalog/ChangeTracking/Language IndexBuilder extensions (keep IConventionIndexBuilder overloads with renamed params) - Collapse ContainsTable/FreeTextTable overloads by moving columnSelector after search text and making it optional - Update annotation code generator, resource strings, and all tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR applies SQL Server API review follow-ups for EF Core 11.0 around full-text and vector indexing, including API renames, removal of IndexBuilder-based full-text configuration extensions, and a simplified TVF surface for FREETEXTTABLE/CONTAINSTABLE. It also moves FullTextSearchResult into the .Query namespace and updates generators/resources/tests accordingly.
Changes:
- Rename fluent builder methods for full-text (
UseKeyIndex,UseCatalog,HasChangeTracking,UseLanguage) and vector indexes (HasMetric,HasType), and update tests/resources. - Remove
IndexBuilderfull-text extension methods (keepingIConventionIndexBuildervariants) and update the annotation code generator output. - Collapse
FreeTextTable/ContainsTableoverloads by reordering and makingcolumnSelectoroptional; update translation tests.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/EFCore.SqlServer.Tests/Infrastructure/SqlServerModelValidatorTest.cs | Updates vector/full-text builder method names in model validation tests. |
| test/EFCore.SqlServer.Tests/Design/Internal/SqlServerAnnotationCodeGeneratorTest.cs | Updates expected fluent API method names produced by the annotation generator. |
| test/EFCore.SqlServer.FunctionalTests/Scaffolding/CompiledModelSqlServerTest.cs | Updates compiled model scaffolding tests for renamed builder methods. |
| test/EFCore.SqlServer.FunctionalTests/Query/Translations/FullTextSearchTranslationsSqlServerTest.cs | Updates TVF usage to new parameter order (searchText first, optional selector). |
| test/EFCore.SqlServer.FunctionalTests/ModelBuilding/SqlServerModelBuilderTestBase.cs | Updates fluent API tests and test builder wrappers for renamed methods. |
| test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs | Updates migration-model building tests to renamed methods. |
| src/EFCore.SqlServer/Properties/SqlServerStrings.resx | Updates user-facing messages to reference renamed APIs. |
| src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs | Regenerated/designer updates for renamed API references in XML docs. |
| src/EFCore.SqlServer/Metadata/Builders/SqlServerVectorIndexBuilder`1.cs | Renames vector index builder methods to HasMetric/HasType. |
| src/EFCore.SqlServer/Metadata/Builders/SqlServerVectorIndexBuilder.cs | Renames vector index builder methods to HasMetric/HasType. |
| src/EFCore.SqlServer/Metadata/Builders/SqlServerFullTextIndexBuilder`1.cs | Renames full-text index builder methods/parameter names per API review. |
| src/EFCore.SqlServer/Metadata/Builders/SqlServerFullTextIndexBuilder.cs | Renames full-text index builder methods/parameter names per API review. |
| src/EFCore.SqlServer/Extensions/SqlServerQueryableExtensions.cs | Collapses TVF overloads by reordering/making selector optional; updates docs and provider checks. |
| src/EFCore.SqlServer/Extensions/SqlServerIndexBuilderExtensions.cs | Removes IndexBuilder full-text extensions; keeps IConventionIndexBuilder APIs with renamed params. |
| src/EFCore.SqlServer/Extensions/FullTextSearchResult.cs | Moves FullTextSearchResult into Microsoft.EntityFrameworkCore.Query and updates cref signatures. |
| src/EFCore.SqlServer/Design/Internal/SqlServerAnnotationCodeGenerator.cs | Updates fluent API generation to emit renamed full-text builder methods. |
Files not reviewed (1)
- src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs: Language not supported
Comment on lines
162
to
+176
| private static readonly MethodInfo IndexHasFullTextKeyIndexMethodInfo | ||
| = typeof(SqlServerIndexBuilderExtensions).GetRuntimeMethod( | ||
| nameof(SqlServerIndexBuilderExtensions.HasFullTextKeyIndex), [typeof(IndexBuilder), typeof(string)])!; | ||
| = typeof(SqlServerFullTextIndexBuilder).GetRuntimeMethod( | ||
| nameof(SqlServerFullTextIndexBuilder.UseKeyIndex), [typeof(string)])!; | ||
|
|
||
| private static readonly MethodInfo IndexHasFullTextCatalogMethodInfo | ||
| = typeof(SqlServerIndexBuilderExtensions).GetRuntimeMethod( | ||
| nameof(SqlServerIndexBuilderExtensions.HasFullTextCatalog), [typeof(IndexBuilder), typeof(string)])!; | ||
| = typeof(SqlServerFullTextIndexBuilder).GetRuntimeMethod( | ||
| nameof(SqlServerFullTextIndexBuilder.UseCatalog), [typeof(string)])!; | ||
|
|
||
| private static readonly MethodInfo IndexHasFullTextChangeTrackingMethodInfo | ||
| = typeof(SqlServerIndexBuilderExtensions).GetRuntimeMethod( | ||
| nameof(SqlServerIndexBuilderExtensions.HasFullTextChangeTracking), [typeof(IndexBuilder), typeof(FullTextChangeTracking)])!; | ||
| = typeof(SqlServerFullTextIndexBuilder).GetRuntimeMethod( | ||
| nameof(SqlServerFullTextIndexBuilder.HasChangeTracking), [typeof(FullTextChangeTracking)])!; | ||
|
|
||
| private static readonly MethodInfo IndexHasFullTextLanguageMethodInfo | ||
| = typeof(SqlServerIndexBuilderExtensions).GetRuntimeMethod( | ||
| nameof(SqlServerIndexBuilderExtensions.HasFullTextLanguage), [typeof(IndexBuilder), typeof(string), typeof(string)])!; | ||
| = typeof(SqlServerFullTextIndexBuilder).GetRuntimeMethod( | ||
| nameof(SqlServerFullTextIndexBuilder.UseLanguage), [typeof(string), typeof(string)])!; |
There was a problem hiding this comment.
SqlServerAnnotationCodeGenerator now uses MethodInfos for instance methods on SqlServerFullTextIndexBuilder (e.g. UseKeyIndex, UseCatalog). The migrations snapshot generator applies index annotation fluent API calls to an IndexBuilder created via EntityTypeBuilder.HasIndex(...); chaining these instance methods onto an IndexBuilder will produce uncompilable snapshot code. Either keep generating static/extension methods that target IndexBuilder, or (preferred given the API removal) introduce a SQL Server-specific snapshot generator that emits HasFullTextIndex(...) (returning SqlServerFullTextIndexBuilder) for full-text indexes and then chains these instance methods; ensure design-time services register it.
Comment on lines
+8
to
+9
| /// <see cref="SqlServerQueryableExtensions.FreeTextTable{T, TKey}(DbSet{T}, string, Expression{Func{T, object}}?, string?, int?)" /> or | ||
| /// <see cref="SqlServerQueryableExtensions.ContainsTable{T, TKey}(DbSet{T}, string, Expression{Func{T, object}}?, string?, int?)" />. |
There was a problem hiding this comment.
FullTextSearchResult was moved to the Microsoft.EntityFrameworkCore.Query namespace, but there are still internal SQL Server query pipeline references using the unqualified type name (e.g. SqlServerQueryableMethodTranslatingExpressionVisitor), which will no longer compile without updating the namespace/imports. Please update remaining references to the new namespace (add using Microsoft.EntityFrameworkCore.Query; or fully-qualify).
Suggested change
| /// <see cref="SqlServerQueryableExtensions.FreeTextTable{T, TKey}(DbSet{T}, string, Expression{Func{T, object}}?, string?, int?)" /> or | |
| /// <see cref="SqlServerQueryableExtensions.ContainsTable{T, TKey}(DbSet{T}, string, Expression{Func{T, object}}?, string?, int?)" />. | |
| /// <see cref="Microsoft.EntityFrameworkCore.SqlServerQueryableExtensions.FreeTextTable{T, TKey}(DbSet{T}, string, Expression{Func{T, object}}?, string?, int?)" /> or | |
| /// <see cref="Microsoft.EntityFrameworkCore.SqlServerQueryableExtensions.ContainsTable{T, TKey}(DbSet{T}, string, Expression{Func{T, object}}?, string?, int?)" />. |
Member
Author
There was a problem hiding this comment.
@copilot please add using Microsoft.EntityFrameworkCore.Query; as necessary
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the @roji-tagged items from #37993:
FullTextSearchResulttoMicrosoft.EntityFrameworkCore.QuerynamespaceSqlServerFullTextIndexBuilder:HasKeyIndex→UseKeyIndex,OnCatalog→UseCatalog,WithChangeTracking→HasChangeTracking,HasLanguage→UseLanguage; removeNamesuffix from paramsSqlServerVectorIndexBuilder:UseMetric→HasMetric,UseType→HasTypeHasFullTextKeyIndex/HasFullTextCatalog/HasFullTextChangeTracking/HasFullTextLanguageIndexBuilderextension methods (keepIConventionIndexBuilderoverloads with renamed params)ContainsTable/FreeTextTableoverloads by movingcolumnSelectorafter search text and making it optionalTwo items from the issue are deferred for separate investigation:
EntityFrameworkQueryableExtensions(Feb 24)Part of #37993