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

Add support for persisting key properties on JSON-mapped entity types #28594

Open
Tracked by #22953
AndriySvyryd opened this issue Aug 4, 2022 · 2 comments
Open
Tracked by #22953

Comments

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Aug 4, 2022

And make the ordinal property more flexible (e.g. different position in the key or no ordinal at all (the client side collection would need to be a List to preserve the order))

@maumar
Copy link
Contributor

maumar commented May 6, 2023

sending for retriage since our plans for json + value types

@roji
Copy link
Member

roji commented Apr 29, 2024

Some notes on this based on our recent design discussion in Redmond:

Advantages

  • One value of this feature is stable partial updating of (ordered) lists within JSON documents.
    • With a list of complex types, any update/removal of an element can only occur given its index position.
    • If we implement partial updating of collections (as opposed to sending the full list on each update), then Index-based updates are problematic for concurrency: if two UoW sessions apply "delete the first position" concurrently, the first two elements of the list are deleted instead of just the first. A key-based delete would always only delete the desired instance.
    • Similar to other concurrency issues, this can be mitigated by having a concurrency token (on the containing entity type); but not having one is more likely to be a pit of failure for partial updates to lists than other cases.
    • Note that a similar problem already exists with primitive collections, where no key is possible.
  • Similarly, persistent keys would allow stable pointing to an element of a JSON list.
    • Note that this is useful only for elements within a JSON list. For JSON types not within a list, it's already possible to point to them (and update/delete them) without a key, so complex types are sufficient there.
  • Note that a user may decide that the position in the list is a suitable "identifier".
    • This includes usage scenarios where deletion and insertion is rare, and the array position has "meaning".
    • In these cases, simply using a list of complex types is the right solution.

Issues

  • Many databases lack an easy ability to perform JSON operations based on a property within a list; while JSONPATH allows selecting elements of a list where some property is equal to some value, most databases don't support this.
    • It's probably possible to expand the list to a relational resultset (e.g. OPENJSON) and use regular operators - just as we do to translate LINQ queries over JSON lists.
    • While this should work well for querying (and so with navigations pointing to a specific owned entity embedded in some JSON list), it isn't clear whether this can be done for updating/removing; we'd probably have to extract the position of the element in order to pass it to the JSON modify/remove functions, which at the very least will probably be inefficient.
  • Even for regular querying, there's typically no mechanism for efficiently getting a list element whose (key) property is equal to something - the database would have to scan the list. This is in contrast to database primary keys, which are always back by an index.
  • Similarly, there is also no database uniqueness guarantee of the key properties.
    • It's worth remembering that for everything to work properly, keys must be unique across the elements (Posts) across all container entities (Blogs), which will likely be impossible to guarantee.
    • For example, one user could insert a Blog with a Post of key X, and another user could insert another Blog with another Post which also has the key X. At this point, the database is in a problematic state: if some user now loads both blogs, EF's change tracking will throw.
  • Our current modeling doesn't allow for a complex type to contain an (owned) entity type.
    • This means that if some user just wants to have some Posts with persistent keys within a JSON document, its containers inside the JSON must also have persistent keys all the way up - even if there's no such natural key.
    • This is a problem specifically when the containing chain to the JSON root contains non-lists; non-lists already provide stability, and keys are totally redundant - it's perfectly reasonable to reference a customer's "Shipping address" without that address having to have a persistent key (one can think of it as sharing the customer's key).
    • This problem could be mitigated by resorting to synthetic keys for the containing types (just like today), but those are problematic in various ways and force different semantics, just because the type needs to contain a list with persistent keys. A modeling where some structural types have keys and others don't - as opposed to today's stronger distinction between entity and complex types - could help, but would be a big change.

@AndriySvyryd @ajcvickers let me know if I've missed something.

ajcvickers added a commit that referenced this issue Aug 19, 2024
Fixes #29380

There are several aspects to #29380:
- Constructor binding (already fixed)
- Round-tripping the `Id` property value (addressed by this PR)
- Persisting key values in JSON collection documents (tracked by #28594)

I investigated persisting key values, but identity lookup requires key values in the materializer before we do the parsing of the document. This means persisted key values are not available without re-writing this part of the shaper, which we can't do for 9.

To fix the main issue (round-trip `Id`) this PR changes the way identity on owned JSON collection documents works. Instead of discovering and using the `Id` property, we treat it like any other property. We then create a shadow `__synthesizedOrdinal` property to act as the ordinal part of the key.

We need to discuss:
- Is this breaking for any scenario that was working before?
- Does this put us in a bad place for the future of owned types with explicit keys?
ajcvickers added a commit that referenced this issue Aug 20, 2024
Fixes #29380

There are several aspects to #29380:
- Constructor binding (already fixed)
- Round-tripping the `Id` property value (addressed by this PR)
- Persisting key values in JSON collection documents (tracked by #28594)

I investigated persisting key values, but identity lookup requires key values in the materializer before we do the parsing of the document. This means persisted key values are not available without re-writing this part of the shaper, which we can't do for 9.

To fix the main issue (round-trip `Id`) this PR changes the way identity on owned JSON collection documents works. Instead of discovering and using the `Id` property, we treat it like any other property. We then create a shadow `__synthesizedOrdinal` property to act as the ordinal part of the key.

We need to discuss:
- Is this breaking for any scenario that was working before?
- Does this put us in a bad place for the future of owned types with explicit keys?
ajcvickers added a commit that referenced this issue Aug 20, 2024
* Persist `Id` values into owned collection JSON documents

Fixes #29380

There are several aspects to #29380:
- Constructor binding (already fixed)
- Round-tripping the `Id` property value (addressed by this PR)
- Persisting key values in JSON collection documents (tracked by #28594)

I investigated persisting key values, but identity lookup requires key values in the materializer before we do the parsing of the document. This means persisted key values are not available without re-writing this part of the shaper, which we can't do for 9.

To fix the main issue (round-trip `Id`) this PR changes the way identity on owned JSON collection documents works. Instead of discovering and using the `Id` property, we treat it like any other property. We then create a shadow `__synthesizedOrdinal` property to act as the ordinal part of the key.

We need to discuss:
- Is this breaking for any scenario that was working before?
- Does this put us in a bad place for the future of owned types with explicit keys?

* Added validation of no explicit keys
ajcvickers added a commit that referenced this issue Aug 20, 2024
* Persist `Id` values into owned collection JSON documents

Fixes #29380

There are several aspects to #29380:
- Constructor binding (already fixed)
- Round-tripping the `Id` property value (addressed by this PR)
- Persisting key values in JSON collection documents (tracked by #28594)

I investigated persisting key values, but identity lookup requires key values in the materializer before we do the parsing of the document. This means persisted key values are not available without re-writing this part of the shaper, which we can't do for 9.

To fix the main issue (round-trip `Id`) this PR changes the way identity on owned JSON collection documents works. Instead of discovering and using the `Id` property, we treat it like any other property. We then create a shadow `__synthesizedOrdinal` property to act as the ordinal part of the key.

We need to discuss:
- Is this breaking for any scenario that was working before?
- Does this put us in a bad place for the future of owned types with explicit keys?

* Added validation of no explicit keys
ajcvickers added a commit that referenced this issue Aug 21, 2024
* Persist `Id` values into owned collection JSON documents

Fixes #29380

There are several aspects to #29380:
- Constructor binding (already fixed)
- Round-tripping the `Id` property value (addressed by this PR)
- Persisting key values in JSON collection documents (tracked by #28594)

I investigated persisting key values, but identity lookup requires key values in the materializer before we do the parsing of the document. This means persisted key values are not available without re-writing this part of the shaper, which we can't do for 9.

To fix the main issue (round-trip `Id`) this PR changes the way identity on owned JSON collection documents works. Instead of discovering and using the `Id` property, we treat it like any other property. We then create a shadow `__synthesizedOrdinal` property to act as the ordinal part of the key.

We need to discuss:
- Is this breaking for any scenario that was working before?
- Does this put us in a bad place for the future of owned types with explicit keys?

* Added validation of no explicit keys
ajcvickers added a commit that referenced this issue Aug 21, 2024
* Persist `Id` values into owned collection JSON documents

Fixes #29380

There are several aspects to #29380:
- Constructor binding (already fixed)
- Round-tripping the `Id` property value (addressed by this PR)
- Persisting key values in JSON collection documents (tracked by #28594)

I investigated persisting key values, but identity lookup requires key values in the materializer before we do the parsing of the document. This means persisted key values are not available without re-writing this part of the shaper, which we can't do for 9.

To fix the main issue (round-trip `Id`) this PR changes the way identity on owned JSON collection documents works. Instead of discovering and using the `Id` property, we treat it like any other property. We then create a shadow `__synthesizedOrdinal` property to act as the ordinal part of the key.

We need to discuss:
- Is this breaking for any scenario that was working before?
- Does this put us in a bad place for the future of owned types with explicit keys?

* Added validation of no explicit keys
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants