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

Query: Define set of conditions for materialization on EntityShaper #20332

Closed
Tracked by #18923
smitpatel opened this issue Mar 18, 2020 · 2 comments · Fixed by #20321, #20324, #20325 or #20345
Closed
Tracked by #18923

Query: Define set of conditions for materialization on EntityShaper #20332

smitpatel opened this issue Mar 18, 2020 · 2 comments · Fixed by #20321, #20324, #20325 or #20345
Assignees
Labels
area-query closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@smitpatel
Copy link
Contributor

smitpatel commented Mar 18, 2020

  • Entity is not coming from left join, has a key and key value contains null then we throw
  • Entity is coming from left join, has a key and key value contains null then return null
  • Entity has valid key and is not optional dependent which is sharing same table, then we materialize (this cover required dependents too)
  • Entity is optional dependent with table splitting and has a valid key, then we materialize if excluding PK, all of the required columns have value or any of optional column has non-null value else we return null
  • Entity is keyless, coming from left join and all properties are null then we return null, we materialize all other keyless entities

In addition to above to support #10140 the flexible mapping condition would become part of it.

EntityShaper will contain a lambda of Func<ValueBuffer, IEntityType>
The lambda returns IEntityType based on above set of conditions telling which entityType to materialize. If there is not entityType should be materialized (e.g. optional dependent), then it returns null. If there is no matching entityType and null cannot be returned (basically unknown discriminator value), it throws exception.

Core EntityShaper contains conditions for discriminator/keyless entityType.
Relational EntityShaper will extend and add conditions for required/optional dependents when table splitting.

@AndriySvyryd - Can you review conditions at the start if that matches what we decided in meeting?

@smitpatel
Copy link
Contributor Author

Necessary client side component for #18299

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Mar 19, 2020

Looks right, the other thing we discussed was that in some cases we need to determine what shaper to use for the dependent entities that aren't related between them other that being mapped to the same table and the same columns.

  • If the principal entities are not being projected we could either use joins to separate them into different columns (current approach, produces more nulls in the reader) or to add a column to the result that would discriminate them (usually the discriminator column for the principal, but could also be a synthesized value).
  • If the query also projects the principal entities (the common case) we could materialize the principal first and use the result (specifically its IEntityType) to determine the correct dependent shaper to invoke.
    To support Allow related entities to be passed to constructor of aggregate root #12078 where the dependents are materialized first we would need to either fallback to joins, prevent this kind of mapping in model validation or read the principal column values to determine its IEntityType before materializing the dependents.

smitpatel added a commit that referenced this issue Mar 19, 2020
Resolves #18299

Fix involves:
- Adding additional materialization conditions for relational layer when table splitting is used. See #20332 for conditions
- Create EntityProjection using the same table when both types are sharing the table.
- Fix a bug in shaper when null entityType is returned from materialization condition, don't call StartTracking.
- Assign nullability to PK column correctly for optional dependents so we don't project it twice.
- Disabled Sql assertion in OwnedQuerySqlServerTest. Will enable after model update as described in #20336, #20343
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Mar 19, 2020
smitpatel added a commit that referenced this issue Mar 19, 2020
Fix involves:
- Adding additional materialization conditions for relational layer when table splitting is used. See #20332 for conditions
- Create EntityProjection using the same table when both types are sharing the table.
- Fix a bug in shaper when null entityType is returned from materialization condition, don't call StartTracking.
- Assign nullability to PK column correctly for optional dependents so we don't project it twice.
- Disabled Sql assertion in OwnedQuerySqlServerTest. Will enable after model update as described in #20336, #20343

Resolves #18299
Resolves #20338
Resolves #20332
smitpatel added a commit that referenced this issue Mar 20, 2020
Fix involves:
- Adding additional materialization conditions for relational layer when table splitting is used. See #20332 for conditions
- Create EntityProjection using the same table when both types are sharing the table.
- Fix a bug in shaper when null entityType is returned from materialization condition, don't call StartTracking.
- Assign nullability to PK column correctly for optional dependents so we don't project it twice.
- Disabled Sql assertion in OwnedQuerySqlServerTest. Will enable after model update as described in #20336, #20343

Resolves #18299
Resolves #20338
Resolves #20332
@ajcvickers ajcvickers modified the milestones: 5.0.0, 5.0.0-preview3 Mar 31, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0-preview3, 5.0.0 Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment