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

stop using local_rels/attrs in schema update #5595

Open
wants to merge 8 commits into
base: stable
Choose a base branch
from

Conversation

ajtmccarty
Copy link
Contributor

@ajtmccarty ajtmccarty commented Jan 27, 2025

IFC-1090

made a followup issue to remove duplicated inherited relationship/attribute data from the database: IFC-1201

loading an extension of a schema that inherits a relationship from a generic can cause the inherited relationship on the schema to be deleted.

the code seems to indicate that we assume a given SchemaNode in the database will only have its non-inherited ("local") relationships linked and will NOT have any inherited relationships, but this is not how it actually works at the database level. a SchemaNode will be connected to SchemaRelationships for every one of its relationships, whether they are inherited from a generic or not

an example from the database
the red path is for the interfaces relationship on a generic schema and the green path is for the inherited interfaces relationship on a node schema. both the SchemaGeneric and SchemaNode include this relationship even though it is only defined on the generic
image

my fix for this issue is to consider all relationships and attributes when updating a schema for a node instead of just the inherited/local ones. this does fix the problem, but I am worried that there may be side effects that are not covered by our current test suite b/c we don't have a lot of tests for loading/updating schema

@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Jan 27, 2025
Copy link

codspeed-hq bot commented Jan 27, 2025

CodSpeed Performance Report

Merging #5595 will not alter performance

Comparing ajtm-01272025-generic-extensions-bug (d54ddf3) with stable (be13145)

Summary

✅ 11 untouched benchmarks

@ajtmccarty ajtmccarty force-pushed the ajtm-01272025-generic-extensions-bug branch from c7250f3 to 9d1802b Compare January 29, 2025 02:39
@ajtmccarty ajtmccarty marked this pull request as ready for review January 29, 2025 04:40
@ajtmccarty ajtmccarty requested a review from a team January 29, 2025 04:40
@LucasG0
Copy link
Contributor

LucasG0 commented Jan 30, 2025

Changes seem to affect any node update, not specifically a node loaded through extensions, it makes me wonder:

  • Would these changes also fix other cases not related to extensions?
  • Or should the fix be located around some extensions related code?

I am not familiar with extensions schemas though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
group/backend Issue related to the backend (API Server, Git Agent)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants