8000 Discussion: The need for helper methods which make removing relationships possible · Issue #219 · src-d/go-kallax · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Discussion: The need for helper methods which make removing relationships possible #219

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.

Sign up for GitHub

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

Jump to bottom
Open
nadiamoe opened this issue Aug 2, 2017 · 3 comments
Open

Discussion: The need for helper methods which make removing relationships possible #219

nadiamoe opened this issue Aug 2, 2017 · 3 comments
Labels
enhancement

Comments

@nadiamoe
Copy link
Contributor
nadiamoe commented Aug 2, 2017

Now that #211 is fixed and Save() does not delete the relations of fields set to nil, the need of direct relationship manipulation may arise in some scenarios.

Consider, for example, the following two entities

type Person struct {
    kallax.Model
    Name string
    Dogs []*Dog
}

type Dog struct {
    kallax.Model
    Name string
    Owner *Person
}

If our person donald is a very bad guy, he might want to abandon his dog. Setting snoopy.Owner to nil won't actually remove the relation, just like removing snoopy from donald.Pets and then issuing an update.

Since raw queries are something that (at least I think) should be avoided, giving DogStore functions to manipulate the owner directly (SetOwner(p *Person, d... *Dog)?) might be a good thing.

This example may look as a corner case, as often models cannot exist without relations, but for more complex models which are expensive to create, or those applications which choose to implement some sort of "soft delete" It might be a good thing for kallax to have.
@nadiamoe
Copy link
Contributor Author
nadiamoe commented Aug 2, 2017

It might also be worth mentioning that if N:M relationships are to bee implemented soon, there should be a way to remove them. As doing this by deleting the desired element from the slice is not an option, this methods could provide the solutions to both problems easily.

@nadiamoe nadiamoe mentioned this issue Aug 24, 2017
Closed
@niondir
Copy link
Contributor
niondir commented Aug 24, 2017 •
edited
Loading

Issuing a query that explicitly updates the relation column could be used to delete the relation. I guess you can assume that a programmer who explicitly states a nil value column of an Entity inside an store.Save() or store.Update() query is willing to actually set the field to nil and thus remove the relation.
Where someone who just fetched an object with the relation implicitly set to nil probably want to maintain the relation on a generic store.Save() or store.Update() call (thats why #211 is a bug ;) ).

Another idea is to allow checking the relation key after some query that does not fetch the relation. I might want to get the ID of dog.Owner without actually fetching data from the Owner table.
So the entity struct could have a property like dog.OwnerID() automatically generated.
This could be another way to explicitly clear some relation with a method like dog.ClearOwnerID() or even dog.SetOwnerID(), but the latter one could lead to inconsistencies with the dog.Owner field which must be avoided.

Just some thoughts :)

@nadiamoe
Copy link
Contributor Author

So, if I understood properly, your first suggestion is to make Update() forcefully update a relationship (even to null) if the FK is explicitly passed in the cols slice. This is kind of related to #233, as for now the behaviour of Update() is to update relations regardless the cols passed.

The main advantage of this approach is we won't need to implement new functions, just tweak existing ones. However, this can also be a bit less intuitive. It just doesn't come to your mind that Update(thing) and Update(thing, Schema.Thing.Field) can lead to a different operation being performed. I mean, it could be clarified on the docs, but I think explicit methods are a better idea.

I like to think I'm good at naming things, but I can't think of anything but UnrelateDog() and UnlinkDog(). I slightly prefer the first one.

Btw, I think that reading (but not writing) the ID can be done with dog.Value(Schema.Dog.OwnerFK.String()). Which is kind of hacky, but still.

To sum up, yes, we could tweak Update(), but it can be confusing. I think creating new methods is the preferred way. It also can be more coherent if N:M relations are implemented at some point.

@niondir niondir mentioned this issue Aug 29, 2017
Open
@nadiamoe nadiamoe mentioned this issue Nov 24, 2017
Open
@nadiamoe nadiamoe mentioned this issue Dec 30, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@niondir @nadiamoe @erizocosmico
0