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

Temporal Table: Owned Entities support? #26451

Closed
JonneOkkonen opened this issue Oct 25, 2021 · 0 comments · Fixed by #26935
Closed

Temporal Table: Owned Entities support? #26451

JonneOkkonen opened this issue Oct 25, 2021 · 0 comments · Fixed by #26935
Assignees
Labels
area-query area-temporal-tables closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Milestone

Comments

@JonneOkkonen
Copy link

I have been testing the new Temporal Table support in EF Core 6 and I just found out that it doesn't seem to work properly with Owned entity types. Adding/updating/removing entities that have owned entities work just fine, but when I try to get history data with TemporalAll/TemporalAsOf, it will return the history correctly for the Main Entity but not for the Owned Entity. Returned Owned Entity values are current values, not historical values. If you will look at the generated SQL below, you can see that the FOR SYSTEM_TIME AS OF is missing from LEFT JOIN and that's why it's not getting history data for Owned Entities. Is this intended functionality or bug?

I have linked the test project below if it would be of any use.

Main Entity Structure

MainEntity mainEntity = new()
{
    Description = "Main entity",
    OwnedEntity = new()
    {
        Description = "Owned entity"
    }
};

TemporalAsOf generated query

SELECT
    [m].[Id], [m].[Description], [m].[EndTime], [m].[StartTime], 
    [o].[MainEntityId], [o].[Description], [o].[EndTime], [o].[StartTime]
FROM [MainEntity] FOR SYSTEM_TIME AS OF '2021-10-25T07:57:11.4857200' AS [m]
LEFT JOIN [OwnedEntity] AS [o] ON [m].[Id] = [o].[MainEntityId]

EF Core version: 6.0.0-rtm.21519.8
Database provider: Microsoft.EntityFrameworkCore.SqlServer (SQL Server 15.0.4153)
Target framework: .NET 6
Operating system: Win10
IDE: Visual Studio 2022 17.0
Link to test project

@maumar maumar assigned maumar and unassigned maumar Oct 26, 2021
@ajcvickers ajcvickers added this to the 6.0.x milestone Oct 26, 2021
@smitpatel smitpatel removed this from the 6.0.x milestone Oct 27, 2021
maumar added a commit that referenced this issue Nov 3, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.
maumar added a commit that referenced this issue Nov 4, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.
maumar added a commit that referenced this issue Nov 4, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.
maumar added a commit that referenced this issue Nov 5, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.
maumar added a commit that referenced this issue Nov 5, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.
maumar added a commit that referenced this issue Nov 5, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.
maumar added a commit that referenced this issue Nov 11, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.
maumar added a commit that referenced this issue Nov 15, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.
maumar added a commit that referenced this issue Nov 16, 2021
…ent tables

Fix to #26451 - Temporal Table: Owned Entities support
Fix to #26469 - Query: enable temporal tables for table splitting scenarios
Fix to #26705 - Query: model building for temporal table with explicitly defined period columns could fail if conventions are executed in a delayed fashion

Just like any other nav expansion, this only works for AsOf operation.

Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
We also need to match TableExpression information with ITable metadata, for provider specific table-like expressions.

Relaxed validation for table splitting when the table is temporal - it's now allowed as long as all (base type) entities mapped to this table have congruent period and history table mappings.

Column period property is now created explicitly in the temporal table builder rather than relying on conventions to avoid scenario when conventions are delayed and the period property they are supposed to create is not ready by the time we need it.
maumar added a commit that referenced this issue Nov 16, 2021
…ent tables

Fix to #26451 - Temporal Table: Owned Entities support
Fix to #26469 - Query: enable temporal tables for table splitting scenarios
Fix to #26705 - Query: model building for temporal table with explicitly defined period columns could fail if conventions are executed in a delayed fashion

Just like any other nav expansion, this only works for AsOf operation.

Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
We also need to match TableExpression information with ITable metadata, for provider specific table-like expressions.

Relaxed validation for table splitting when the table is temporal - it's now allowed as long as all (base type) entities mapped to this table have congruent period and history table mappings.

Column period property is now created explicitly in the temporal table builder rather than relying on conventions to avoid scenario when conventions are delayed and the period property they are supposed to create is not ready by the time we need it.
maumar added a commit that referenced this issue Nov 16, 2021
…ent tables

Fix to #26451 - Temporal Table: Owned Entities support
Fix to #26469 - Query: enable temporal tables for table splitting scenarios
Fix to #26705 - Query: model building for temporal table with explicitly defined period columns could fail if conventions are executed in a delayed fashion

Just like any other nav expansion, this only works for AsOf operation.

Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
We also need to match TableExpression information with ITable metadata, for provider specific table-like expressions.

Relaxed validation for table splitting when the table is temporal - it's now allowed as long as all (base type) entities mapped to this table have congruent period and history table mappings.

Column period property is now created explicitly in the temporal table builder rather than relying on conventions to avoid scenario when conventions are delayed and the period property they are supposed to create is not ready by the time we need it.
maumar added a commit that referenced this issue Nov 19, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.

Also added internal interface to TableExpression, so that we can extract table metadata information (table name and schema) from provider specific table expressions.
maumar added a commit that referenced this issue Nov 22, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.

Also added internal interface to TableExpression, so that we can extract table metadata information (table name and schema) from provider specific table expressions.
maumar added a commit that referenced this issue Nov 23, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.

Also added internal interface to TableExpression, so that we can extract table metadata information (table name and schema) from provider specific table expressions.
maumar added a commit that referenced this issue Nov 23, 2021
Added extensibility point to SharedTypeEntityExpandingExpressionVisitor so that we can create temporal table expressions representing owned entities, if their owner is also a temporal table expression.
Just like any other nav expansion, this only works for AsOf operation.

Also added internal interface to TableExpression, so that we can extract table metadata information (table name and schema) from provider specific table expressions.
maumar added a commit that referenced this issue Dec 8, 2021
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)

Fixes #26858
Fixes #26469
Fixes #26451
maumar added a commit that referenced this issue Dec 8, 2021
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)

Fixes #26858
Fixes #26469
Fixes #26451
maumar added a commit that referenced this issue Dec 8, 2021
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)

Fixes #26858
Fixes #26469
Fixes #26451
maumar added a commit that referenced this issue Dec 9, 2021
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)

Fixes #26858
Fixes #26469
Fixes #26451
maumar added a commit that referenced this issue Dec 9, 2021
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)

Fixes #26858
Fixes #26469
Fixes #26451
maumar added a commit that referenced this issue Dec 9, 2021
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)

Fixes #26858
Fixes #26469
Fixes #26451
@ajcvickers ajcvickers added this to the 7.0.0 milestone Dec 9, 2021
maumar added a commit that referenced this issue Dec 10, 2021
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)

Fixes #26858
Fixes #26469
Fixes #26451
maumar added a commit that referenced this issue Dec 16, 2021
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)

Fixes #26858
Fixes #26469
Fixes #26451
maumar added a commit that referenced this issue Dec 17, 2021
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)

Fixes #26858
Fixes #26469
Fixes #26451
maumar added a commit that referenced this issue Jan 13, 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)

Fixes #26858
Fixes #26469
Fixes #26451
maumar added a commit that referenced this issue Jan 13, 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)

Fixes #26858
Fixes #26469
Fixes #26451
maumar added a commit that referenced this issue Jan 13, 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)

Fixes #26858
Fixes #26469
Fixes #26451
@maumar maumar closed this as completed in 1ca6380 Jan 13, 2022
@maumar maumar added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jan 13, 2022
maumar added a commit that referenced this issue 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)
maumar added a commit that referenced this issue Feb 5, 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)
maumar added a commit that referenced this issue Feb 7, 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)
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-preview1 Feb 14, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-preview1, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-query area-temporal-tables closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
4 participants