Allow configuring SQL Server temporal table period columns as not hidden#38225
Open
m-x-shokhzod wants to merge 5 commits intodotnet:mainfrom
Open
Allow configuring SQL Server temporal table period columns as not hidden#38225m-x-shokhzod wants to merge 5 commits intodotnet:mainfrom
m-x-shokhzod wants to merge 5 commits intodotnet:mainfrom
Conversation
Adds TemporalTableBuilder.PeriodColumnsHidden(bool hidden = true) to opt out of the HIDDEN flag on period start/end columns. Default remains HIDDEN to preserve backward compatibility. Fixes dotnet#36608
Period columns are shadow properties at the model level regardless of the HIDDEN SQL flag, so the IColumn collection only exposes Id and Name. The HIDDEN flag affects emitted SQL only, not table.Columns.
Member
|
Good complement to #38110, which we've already done for preview-4. Some quick notes:
/cc @AndriySvyryd |
There was a problem hiding this comment.
Pull request overview
This PR introduces an opt-out for SQL Server temporal table period columns being created as HIDDEN, via new fluent API TemporalTableBuilder.PeriodColumnsHidden(bool hidden = true). This preserves the existing default (HIDDEN) for backwards compatibility while allowing period columns to be visible in SELECT * when explicitly configured.
Changes:
- Adds a new model annotation (
SqlServerAnnotationNames.TemporalPeriodColumnsHidden) and entity-type extension APIs to store/read the setting (defaulting totruewhen not configured). - Extends all temporal table builders (regular/generic + owned navigation regular/generic) with
PeriodColumnsHidden(...). - Propagates the annotation through design-time model annotation generation and SQL generation, adding a functional migrations test and model-building tests.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/EFCore.SqlServer.FunctionalTests/ModelBuilding/SqlServerModelBuilderTestBase.cs | Adds model-building tests asserting default hidden behavior and explicit visibility via PeriodColumnsHidden(false), plus test builder plumbing. |
| test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsSqlServerTest.TemporalTables.cs | Adds a migrations functional test verifying CREATE TABLE SQL omits HIDDEN when configured. |
| src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs | Emits HIDDEN conditionally for period columns based on the new annotation; strips the annotation in some temporal-migration normalization paths. |
| src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationProvider.cs | Flows the “not hidden” configuration into design-time annotations for tables and for the period start/end columns. |
| src/EFCore.SqlServer/Metadata/Internal/SqlServerAnnotationNames.cs | Introduces the TemporalPeriodColumnsHidden annotation name constant. |
| src/EFCore.SqlServer/Metadata/Builders/TemporalTableBuilder`.cs | Adds the generic builder PeriodColumnsHidden(...) (return-type narrowing wrapper). |
| src/EFCore.SqlServer/Metadata/Builders/TemporalTableBuilder.cs | Adds the core temporal table builder API PeriodColumnsHidden(...) to set the new entity-type annotation. |
| src/EFCore.SqlServer/Metadata/Builders/OwnedNavigationTemporalTableBuilder``.cs | Adds the generic owned-navigation builder PeriodColumnsHidden(...) (return-type narrowing wrapper). |
| src/EFCore.SqlServer/Metadata/Builders/OwnedNavigationTemporalTableBuilder.cs | Adds the core owned-navigation temporal table builder API PeriodColumnsHidden(...). |
| src/EFCore.SqlServer/Extensions/SqlServerEntityTypeExtensions.cs | Adds IsTemporalPeriodColumnsHidden / SetIsTemporalPeriodColumnsHidden (mutable + convention) and configuration-source accessors. |
| src/EFCore.SqlServer/EFCore.SqlServer.baseline.json | Updates API baseline for the newly-added public surface area. |
| src/EFCore.SqlServer/Design/Internal/SqlServerCSharpRuntimeAnnotationCodeGenerator.cs | Strips the new annotation from runtime annotation output (consistent with other temporal annotations). |
| src/EFCore.SqlServer/Design/Internal/SqlServerAnnotationCodeGenerator.cs | Adds snapshot fluent API generation for .PeriodColumnsHidden(false) and removes the annotation afterward. |
Comment on lines
+1886
to
+1892
| // Defaults to true to preserve backward compatibility - the period columns have always been hidden. | ||
| // Set to false via TemporalTableBuilder.PeriodColumnsHidden(false) to make them visible. | ||
| var hidden = operation[SqlServerAnnotationNames.TemporalPeriodColumnsHidden] as bool? ?? true; | ||
| if (hidden) | ||
| { | ||
| builder.Append(" HIDDEN"); | ||
| } |
Addresses feedback on PR dotnet#38225: - roji dotnet#1: per-column HIDDEN configuration → IsHidden(bool) on TemporalPeriodPropertyBuilder + OwnedNavigationTemporalPeriodPropertyBuilder lets users hide the start and end columns independently. - roji dotnet#2: HIDDEN as a property facet rather than a temporal-specific entity setting → new SqlServerAnnotationNames.IsHidden plus SqlServerPropertyExtensions IsHidden / SetIsHidden / GetIsHiddenConfigurationSource (mirroring IsSparse). PeriodColumnsHidden(bool) is preserved on TemporalTableBuilder as a convenience that calls SetIsHidden on both period properties. - Copilot bug: convert-to-temporal path was always emitting ALTER COLUMN ... ADD HIDDEN. Two new table-level annotations TemporalPeriodStartHidden / TemporalPeriodEndHidden are emitted by SqlServerAnnotationProvider.For(ITable) when a period property is configured visible. BuildTemporalInformationFromMigrationOperation reads them into TemporalOperationInformation, and EnablePeriod skips the ADD HIDDEN ALTER COLUMN operations when the column is configured visible. - New functional test Convert_normal_table_to_temporal_with_visible_period_columns asserts the ADD HIDDEN operations are omitted. - Snapshot generator chains .IsHidden(false) onto the period property fluent call when the column is configured visible. - Removed entity-level TemporalPeriodColumnsHidden annotation and the IsTemporalPeriodColumnsHidden / SetIsTemporalPeriodColumnsHidden / GetIsTemporalPeriodColumnsHiddenConfigurationSource entity extensions. - Updated baseline.json accordingly.
m-x-shokhzod
force-pushed
the
feat/sqlserver-period-columns-not-hidden
branch
from
6956bbd to
76554e0
Compare
The pair-level convenience PeriodColumnsHidden(bool) on TemporalTableBuilder
had a silent-no-op bug: when called before HasPeriodStart/HasPeriodEnd, the
period property names were not yet set, so the implementation could not find
the properties to call SetIsHidden on.
The property-level IsHidden(bool) on TemporalPeriodPropertyBuilder is order-safe
because it chains directly off HasPeriodStart/HasPeriodEnd which return the
period property builder for the property that was just configured. It also
matches roji's preference for HIDDEN as a property-level facet rather than a
temporal-table-specific entity setting.
- Removed PeriodColumnsHidden from TemporalTableBuilder + generic +
OwnedNavigationTemporalTableBuilder + generic
- Removed corresponding abstract / override methods from test wrappers
- Updated functional tests to chain IsHidden(false) per period property:
ttb.HasPeriodStart("Start").IsHidden(false);
ttb.HasPeriodEnd("End").IsHidden(false);
- Updated baseline.json (4 entries removed)
This also fixes the CI failure on Convert_normal_table_to_temporal_with_visible_period_columns
on SqlServer 2019/2022/2025: the test was using PeriodColumnsHidden(false)
inside ToTable(...IsTemporal(...)) before period property names were set,
making the call a no-op and leaving ALTER COLUMN ... ADD HIDDEN in the
emitted SQL.
In RewriteOperations, the AlterTableOperation case builds the temporal information from alterTableOperation.OldTable to capture the initial (non-temporal) state. But that source has no TemporalPeriodStartHidden / TemporalPeriodEndHidden annotations, so PeriodStartHidden/PeriodEndHidden defaulted to true and EnablePeriod always emitted ALTER COLUMN ... ADD HIDDEN, ignoring the user's IsHidden(false) configuration. Override just those two flags from alterTableOperation itself, which carries the target-state annotations emitted by SqlServerAnnotationProvider. All other initial-state reads from OldTable remain intact.
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.
Adds
TemporalPeriodPropertyBuilder.IsHidden(bool hidden = true)to opt individual SQL Server temporal table period columns out of theHIDDENflag. Default remainsHIDDENto preserve backward compatibility. Each period column is independently configurable.Per @roji's review feedback,
IsHiddenis exposed as a general SQL Server property facet (mirroringIsSparse), not a temporal-specific flag — forward-compatible with future non-temporalHIDDENscenarios.SqlServerAnnotationNames.IsHidden(property + column level)SqlServerAnnotationNames.TemporalPeriodStartHidden/TemporalPeriodEndHidden(table level, lifted bySqlServerAnnotationProviderfor the migrations differ)IsHidden/SetIsHidden(mutable + convention) /GetIsHiddenConfigurationSourceonSqlServerPropertyExtensions(mirrorsIsSparsepattern)IsHidden(bool hidden = true)method added toTemporalPeriodPropertyBuilderandOwnedNavigationTemporalPeriodPropertyBuilderSqlServerAnnotationProvider(per-period-column) andSqlServerMigrationsSqlGenerator— conditionalHIDDENSQL emit in bothColumnDefinition(CREATE TABLE path) andEnablePeriod(ALTER TABLE convert-to-temporal path).IsHidden(false)per period property; runtime annotation strip inSqlServerCSharpRuntimeAnnotationCodeGeneratorMigrationsSqlServerTest.TemporalTables(Create_temporal_table_with_period_columns_not_hidden,Convert_normal_table_to_temporal_with_visible_period_columns) verified against real SQL Server in CI matrix (2019/2022/2025); 3 model-builder tests inSqlServerModelBuilderTestBase; baseline.json entries updated for new public surfaceFixes #36608