Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/6.0] Temporal table support for owned and table splitting scenarios #27369

Closed
wants to merge 1 commit into from

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Feb 4, 2022

Fix to #26858 - Query: improve TableExpressionBase extensibility by adding annotations
Fix to #26469 - Query: enable temporal tables for table splitting scenarios
Fix to #26451 - Temporal Table: Owned Entities support?

Added annotation infra for TableExpressionBase and annotations for temporal table information. Removed (now unnecessary) temporal specific table expressions.
Also added temporal table support for owned typed and table splitting in general using the annotations to store the temporal information (no need for provider specific logic in places where we didn't have good extensibility)
For table splitting, every entity must explicitly define period start/end columns. They all need to be the same, but if not explicitly provided we try to uniquefy them. We should fix this so that you only need to specify it in one place but it's currently hard to do (hopefully convention layering will make it easier)


Description

Temporal table feature was disabled for shared/owned types. This change enables the functionality - it requires refactoring to some sql expressions used by EF internally, as well as slightly modifying model builders to fix bug in nested temporal type declarations using conventions. (which often appears in owned type scenarios)

Customer impact

Customers were unable to use owned/share types with temporal tables feature.

How found

Issue was known to us. Decision based on feedback from multiple customers as well as from partners within Microsoft.

Regression

No. Temporal tables is a new feature in 6.0

Testing

Added extensive testing for owned/shared type scenarios (over 500 test cases) .

Risk

Medium. This is a large change (for a patch) which involves additional API and slightly different mechanism for building temporal table conventions. Added quirk to revert to previous behavior, but it doesn't revert 100% - model builder / convention changes will behave in new way even after the quirk has been applied. The change should only affect temporal table scenarios.

Fix to #26858 - Query: improve TableExpressionBase extensibility by adding annotations
Fix to #26469 - Query: enable temporal tables for table splitting scenarios
Fix to #26451 - Temporal Table: Owned Entities support?

Added annotation infra for TableExpressionBase and annotations for temporal table information. Removed (now unnecessary) temporal specific table expressions.
Also added temporal table support for owned typed and table splitting in general using the annotations to store the temporal information (no need for provider specific logic in places where we didn't have good extensibility)
For table splitting, every entity must explicitly define period start/end columns. They all need to be the same, but if not explicitly provided we try to uniquefy them. We should fix this so that you only need to specify it in one place but it's currently hard to do (hopefully convention layering will make it easier)
@@ -13,6 +14,7 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[Obsolete("Use TableExpressionBase annotations to convey temporal query information")]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant