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/8.0] Fix to #29156 - TemporalAll for temporal owned entities mapped to parent table. #32031

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

maumar
Copy link
Contributor

@maumar maumar commented Oct 11, 2023

Similar to JSON entities, owned entities that are mapped to the same table as their owner should be treated as scalars for the purpose of temporal query validation - they are always in sync with the parent entity, so all operations should be allowed for them, not only AsOf.

Fixes #29156

Risk

Low - this is very similar to the JSON scenario fixed approved for 8.0 recently (see #32006) - we relax the validation we had when encountering navigations with temporal operations. We normally only allow AsOf, otherwise the results are not very useful/coherent, but in case of owned entities mapped to the same table, they are always in sync with parent, so all operations are fine.

@maumar maumar requested a review from a team October 11, 2023 23:27
…ent table.

Similar to JSON entities, owned entities that are mapped to the same table as their owner should be treated as scalars for the purpose of temporal query validation - they are always in sync with the parent entity, so all operations should be allowed for them, not only AsOf.

Fixes #29156
=> entityType.IsOwned()
&& entityType.FindOwnership()!.PrincipalEntityType.GetTableMappings().FirstOrDefault()?.Table is ITable ownerTable
&& entityType.GetTableMappings().FirstOrDefault()?.Table is ITable entityTable
&& ownerTable == entityTable;
Copy link
Member

Choose a reason for hiding this comment

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

Why are you only checking table mappings, wouldn't the same apply for entity types only mapped to views?

Copy link
Contributor Author

@maumar maumar Oct 12, 2023

Choose a reason for hiding this comment

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

we don't support "temporal views" (I don't think it's even a thing) - ViewBuilder doesn't have API to set it to temporal.

@maumar maumar merged commit d0055a4 into release/8.0 Oct 12, 2023
7 checks passed
@maumar maumar deleted the fix29156 branch October 12, 2023 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TemporalAll for temporal owned entities mapped to parent table.
2 participants