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

Cosmos: Validate our current container mapping strategy #34005

Closed
roji opened this issue Jun 17, 2024 · 2 comments
Closed

Cosmos: Validate our current container mapping strategy #34005

roji opened this issue Jun 17, 2024 · 2 comments
Labels
closed-no-further-action The issue is closed and no further action is planned.

Comments

@roji
Copy link
Member

roji commented Jun 17, 2024

We currently map all entity types to the same container by default, and include a discriminator predicate in all queries.

  • Is single-container the correct default, as opposed to e.g. container-per-entity-type (which is more similar to the relational table-per-entity-type strategy)?
    • Some Cosmos docs on containers
    • At some point we think there was a billing consideration that made multiple containers less desirable - is that actually/still the case?
    • One critical thing here seems to be the partition key: "Containers are schema agnostic. Items within a container can have arbitrary schemas or different entities, as long as they share the same partition key". In other words, since a partition key is defined at the container level, if we expect different entity types to require different partition keys (which seems to be the case in general), then we should split each entity type to its own container.
    • Various other things are configured at the container level which don't seem to apply to all entity types in a model: unique keys constraints, index config, etc.
    • Of course, if/when we implement hierarchy mapping, it would make sense for the entire hierarchy to be mapped to the same container (basically TPH).
  • In any case, when a query is generated against a container to which only one entity type is mapped (given complete discriminator mapping, see #20268), we shouldn't add a discriminator predicate to the query.
@roji
Copy link
Member Author

roji commented Jun 24, 2024

Note that the single container we currently map to is by default named after the context CLR type, so e.g. NorthwindContext. If we're changing things in this area, we may want to chop off "Context", which doesn't seem like it belongs in the database.

@AndriySvyryd AndriySvyryd added this to the 9.0.0 milestone Jul 4, 2024
@ajcvickers ajcvickers changed the title Cosmos: Validate our current container/discriminator mapping strategy Cosmos: Validate our current container mapping strategy Jul 8, 2024
@ajcvickers
Copy link
Member

We discussed this at length with the Cosmos team and decided to keep the mapping to a single container. Mapping to many different containers may defeat efficiency on the Cosmos side when used with certain throughput options. It may still be necessary or desirable to model with multiple containers, but we don't do this by convention in EF Core.

@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Jul 20, 2024
@ajcvickers ajcvickers removed this from the 9.0.0 milestone Jul 20, 2024
@ajcvickers ajcvickers added closed-no-further-action The issue is closed and no further action is planned. and removed needs-design type-enhancement area-cosmos area-model-building labels Jul 20, 2024
@ajcvickers ajcvickers removed their assignment Jul 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned.
Projects
None yet
Development

No branches or pull requests

3 participants