Add DATA_COMPRESSION support for primary keys and unique constraints#38434
Open
m-x-shokhzod wants to merge 1 commit into
Open
Add DATA_COMPRESSION support for primary keys and unique constraints#38434m-x-shokhzod wants to merge 1 commit into
m-x-shokhzod wants to merge 1 commit into
Conversation
Fixes dotnet#33145. SQL Server PRIMARY KEY / UNIQUE constraints accept WITH (DATA_COMPRESSION = ...) just like indexes do. This adds first-class support, mirroring the existing index DATA_COMPRESSION and key FILLFACTOR features: - UseDataCompression on KeyBuilder / KeyBuilder<TEntity> / IConventionKeyBuilder (plus CanSetDataCompression), and Get/SetDataCompression on the key metadata. - SqlServerAnnotationProvider attaches the annotation to the unique constraint. The shared IndexOptions already emits DATA_COMPRESSION for AddPrimaryKey / AddUniqueConstraint operations, so the migrations generator needs no change. - Fluent-API, runtime-annotation, and runtime-model code generation handling. As with the index case, DATA_COMPRESSION is not reverse-engineered.
There was a problem hiding this comment.
Pull request overview
Adds first-class SQL Server DATA_COMPRESSION configuration support for primary keys and unique constraints, aligning key/constraint configuration with existing index option support and enabling migrations to emit WITH (DATA_COMPRESSION = ...) for constraints.
Changes:
- Added key-level APIs (
UseDataCompression,Get/SetDataCompression) and wired them into the SQL Server annotation/codegen pipeline. - Updated
SqlServerAnnotationProvider.For(IUniqueConstraint)to surface keyDATA_COMPRESSIONas a constraint annotation so migrations SQL can emit it. - Added unit and functional test coverage for configuring and generating SQL for compressed PK/AK constraints.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/EFCore.SqlServer.Tests/Metadata/SqlServerBuilderExtensionsTest.cs | Adds tests verifying UseDataCompression can be set on keys (generic and non-generic). |
| test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.cs | Adds migration SQL assertion for DATA_COMPRESSION on PK and unique constraints. |
| src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs | Emits DataCompression annotation for IUniqueConstraint based on mapped key configuration. |
| src/EFCore.SqlServer/Metadata/Conventions/SqlServerRuntimeModelConvention.cs | Strips DataCompression from runtime model annotations for keys (consistent with other key facets). |
| src/EFCore.SqlServer/Extensions/SqlServerKeyExtensions.cs | Introduces key extension APIs for DataCompression (including store-object overload). |
| src/EFCore.SqlServer/Extensions/SqlServerKeyBuilderExtensions.cs | Adds fluent configuration (UseDataCompression) and convention APIs for keys. |
| src/EFCore.SqlServer/EFCore.SqlServer.baseline.json | Updates API baseline to include the new public extension methods. |
| src/EFCore.SqlServer/Design/Internal/SqlServerCSharpRuntimeAnnotationCodeGenerator.cs | Removes DataCompression from runtime annotation emission for keys/unique constraints. |
| src/EFCore.SqlServer/Design/Internal/SqlServerAnnotationCodeGenerator.cs | Emits .UseDataCompression(...) fluent API for key annotations when scaffolding code. |
Comment on lines
+179
to
+182
| if (key.GetDataCompression() is { } dataCompression) | ||
| { | ||
| yield return new Annotation(SqlServerAnnotationNames.DataCompression, dataCompression); | ||
| } |
| /// <returns>The <see cref="ConfigurationSource" /> for whether the key uses the fill factor.</returns> | ||
| public static ConfigurationSource? GetFillFactorConfigurationSource(this IConventionKey key) | ||
| => key.FindAnnotation(SqlServerAnnotationNames.FillFactor)?.GetConfigurationSource(); | ||
|
|
Member
There was a problem hiding this comment.
Modify AreCompatibleForSqlServer to check that the configured values that end up on the same table key are not conflicting
| /// <param name="key">The key.</param> | ||
| /// <param name="dataCompression">The value to set.</param> | ||
| public static void SetDataCompression(this IMutableKey key, DataCompressionType? dataCompression) | ||
| => key.SetAnnotation( |
Member
There was a problem hiding this comment.
Use SetOrRemoveAnnotation. Also below.
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.
SQL Server
PRIMARY KEY/UNIQUEconstraints acceptWITH (DATA_COMPRESSION = ...), just like indexes do (#30408 added it for indexes). This adds first-class support for configuring it on keys, mirroring the existing indexDATA_COMPRESSIONand keyFILLFACTORsupport.produces:
SqlServerAnnotationProvider.For(IUniqueConstraint)attaches the annotation; the base generator'sPrimaryKeyConstraint/UniqueConstraintalready call the sharedIndexOptions, which already emitsDATA_COMPRESSION, so the migrations generator itself is unchanged. As with the index case,DATA_COMPRESSIONis not reverse-engineered.Fixes #33145