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

fix(relationship 2.0): handle the case where aspects can have multiple fields with the same relationship type #476

Merged
merged 5 commits into from
Nov 13, 2024

Conversation

jsdonn
Copy link
Contributor

@jsdonn jsdonn commented Nov 13, 2024

Summary

Currently, these is a "bug" where aspects that have multiple relationship fields of the same relationship type will not have all of them persisted properly in the DB. This is because the current logic for extracting relationships from aspect fields keeps all the extracted relationships separate by the field it was derived from. Then, the relationships are ingested sequentially by each field name group. Relationships in 2.0 are ingested using REMOVE_ALL_EDGES_FROM_SOURCE so any earlier relationships will get overwritten by later relationships if the later ones are of the same type as the earlier ones.

The new logic will group them by relationship class. Each relationship class will be processed sequentially. This way, there is never any overlap of types between different relationship groups. So any deletions will only occur on previously existing relationships, not relationships added earlier in the current ingestion.

Testing Done

Updated unit tests

Checklist

@jsdonn jsdonn marked this pull request as ready for review November 13, 2024 02:27
Copy link
Contributor

@yangyangv2 yangyangv2 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for making the quick fix!

@jsdonn jsdonn merged commit af8812b into linkedin:master Nov 13, 2024
2 checks passed
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