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

feat: Replace FieldDescription.RelationType with IsPrimary #2288

Merged

Conversation

AndrewSisley
Copy link
Contributor

@AndrewSisley AndrewSisley commented Feb 7, 2024

Relevant issue(s)

Resolves #2287 #1772

Description

Replaces FieldDescription.RelationType with an IsPrimary boolean.

One notable effect of this that may not be easily visible in the code-diff is that the schema ID will not change depending on whether it is the primary side of a one-one or on the one side of a one-many - now the schema only cares about the shape of the data, not what is on the other side of the relationship.

Commits should be clean, it might be easier reviewing them one by one.

@AndrewSisley AndrewSisley added feature New feature or request area/schema Related to the schema system labels Feb 7, 2024
@AndrewSisley AndrewSisley added this to the DefraDB v0.10 milestone Feb 7, 2024
@AndrewSisley AndrewSisley requested a review from a team February 7, 2024 01:54
@AndrewSisley AndrewSisley self-assigned this Feb 7, 2024
Copy link

codecov bot commented Feb 7, 2024

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (86b59c6) 74.24% compared to head (5813c1b) 74.08%.

❗ Current head 5813c1b differs from pull request most recent head 61053ab. Consider uploading reports for the commit 61053ab to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2288      +/-   ##
===========================================
- Coverage    74.24%   74.08%   -0.16%     
===========================================
  Files          256      256              
  Lines        25789    25695      -94     
===========================================
- Hits         19145    19034     -111     
- Misses        5331     5347      +16     
- Partials      1313     1314       +1     
Flag Coverage Δ
all-tests 74.08% <91.43%> (-0.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
client/descriptions.go 65.38% <100.00%> (-0.52%) ⬇️
db/collection_update.go 71.28% <100.00%> (ø)
db/errors.go 62.26% <ø> (-4.40%) ⬇️
planner/mapper/mapper.go 87.42% <100.00%> (ø)
planner/type_join.go 78.95% <100.00%> (-0.15%) ⬇️
request/graphql/schema/collection.go 91.00% <100.00%> (+0.05%) ⬆️
db/collection.go 67.79% <86.36%> (-1.99%) ⬇️
request/graphql/schema/relations.go 78.41% <85.00%> (-0.76%) ⬇️

... and 8 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86b59c6...61053ab. Read the comment docs.

Copy link
Contributor

@islamaliev islamaliev left a comment

Choose a reason for hiding this comment

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

Looks much cleaner now. Good job!

@AndrewSisley AndrewSisley force-pushed the 2287-relation-type-cleanup branch from d6e7dc1 to 5813c1b Compare February 7, 2024 16:03
@AndrewSisley AndrewSisley changed the title feat: Replace FieldDescription.RelationType with boolean feat: Replace FieldDescription.RelationType with IsPrimary Feb 7, 2024
@AndrewSisley AndrewSisley merged commit 36315e4 into sourcenetwork:develop Feb 7, 2024
28 of 29 checks passed
@AndrewSisley AndrewSisley deleted the 2287-relation-type-cleanup branch February 7, 2024 16:45
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…work#2288)

## Relevant issue(s)

Resolves sourcenetwork#2287 sourcenetwork#1772

## Description

Replaces `FieldDescription.RelationType` with an `IsPrimary` boolean.

One notable effect of this that may not be easily visible in the
code-diff is that the schema ID will not change depending on whether it
is the primary side of a one-one or on the one side of a one-many - now
the schema only cares about the shape of the data, not what is on the
other side of the relationship.
@islamaliev
Copy link
Contributor

found during bug bash, but most likely not directly related #2380

@islamaliev
Copy link
Contributor

bug bash result: tested by creating different types and 1-to-1 and 1-no-many relationships and querying on them. Everything works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/schema Related to the schema system feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up RelationType
2 participants