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

Store redundant fields in references #1527

Closed

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented Nov 23, 2016

References in MongoDB can be just IDs for a referenced document, or they can be full-fledged DBRef objects that can contain additional information. We use this functionality to add a discriminator value to properly hydrate a referenced document.

This PR allows developers to store additional fields from a referenced document within the reference, so it can be used in queries. There are two caveats at this time:

  • The denormalized value won't be updated. This would require us to keep track of incoming references to a document and update all documents referencing ours when such a field is updated. Apart from the potentially long update query, this would only work within the first nesting level of documents due to SERVER-831.
  • The value is not yet used when creating proxy classes. This is because the current implementation of the proxy library doesn't allow for partially initialized proxies. This has been discussed in Lazy-load on a per-property base (removes doctrine proxies, replaced by ProxyManager) orm#1241 and I'd like to add that to ODM 2.0.

@alcaeus alcaeus added the Idea label Nov 23, 2016
@alcaeus alcaeus self-assigned this Nov 23, 2016
Copy link
Member

@malarzm malarzm left a comment

Choose a reason for hiding this comment

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

Should we limit what can be stored in redundant field? I guess putting there EmbedMany field will have some side effects depending on it being initialzed or not

@malarzm malarzm added this to the 1.2 milestone Nov 23, 2016
@malarzm malarzm added Feature and removed Idea labels Nov 23, 2016
@malarzm
Copy link
Member

malarzm commented Nov 23, 2016

Looks cool, putting into 1.2 feature bucket 👍

@alcaeus alcaeus force-pushed the store-redundant-fields-reference branch from 42af3f2 to 41d3234 Compare November 24, 2016 05:50
@malarzm
Copy link
Member

malarzm commented Nov 24, 2016

Leaving a note to future us:

  • need to check if storing association (can't recall if getField from class metadata can give us association) is safe
  • allowing dot notation in redundant fields sounds interesting (not sure how helpful this could be but https://github.com/doctrine/mongodb-odm/pull/970/files introduced ReferencePrimer::parseDotSyntaxForPrimer which may or may not be useful :) )

@alcaeus alcaeus modified the milestones: 2.0.0, 1.2 Dec 18, 2016
@alcaeus
Copy link
Member Author

alcaeus commented Dec 18, 2016

Dot notation would be a bit complicated, since that may trigger a whole bunch of database loads. There's already a potential database load involved when building a DBRef object (e.g. when the referenced document is not initialized and redundant fields are defined). This happens even when the DBRef object is not used for writing, but only for reading (e.g. by being called in Query\Builder::references. Allowing dot notation and thus a whole reference tree would only make the problem worse. This is something I'd like to address, but I'm not sure it can be done in a BC fashion.

@alcaeus
Copy link
Member Author

alcaeus commented Nov 27, 2020

Closing as the PR is horribly stale: created #2252 to track this and will try to revisit this for one of the next 2.x releases.

@alcaeus alcaeus closed this Nov 27, 2020
@alcaeus alcaeus deleted the store-redundant-fields-reference branch November 27, 2020 08:15
@alcaeus alcaeus removed the Feature label Nov 27, 2020
@alcaeus alcaeus removed this from the 2.x milestone Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants