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: add support for ON UPDATE and ON DELETE rules on belongs-to relationships from struct tags #533

Merged
merged 10 commits into from
Jun 8, 2022

Conversation

antipopp
Copy link
Contributor

@antipopp antipopp commented May 13, 2022

I was missing this feature from go-pg and wanted to add it to the codebase. AFAIK belongs-to is the only rel that would benefit from it, as it's the only one that generates a column on the DB. But I still don't know this repo as deeply as I want to, so feel free to correct me.

type User struct {
		ID   int    `bun:",pk,autoincrement"`
		Type string `bun:",pk"`
		Name string
}
type Deck struct {
		ID       int `bun:",pk,autoincrement"`
		UserID   int
		UserType string
		User     *User `bun:"rel:belongs-to,join:user_id=id,join:user_type=type,on_update:cascade,on_delete:set null"`
}

Also, should I tag the release for this?

@EllaFoxo
Copy link

EllaFoxo commented May 13, 2022

Might be a good idea if we could have a test for this in db_test, similar to how foreign key violations are checked: db_test.go#L820

Love the change though, been missing this feature myself

@antipopp
Copy link
Contributor Author

Might be a good idea if we could have a test for this in db_test, similar to how foreign key violations are checked: db_test.go#L820

Love the change though, been missing this feature myself

I've been thinking about the tests. What would a pattern look like? Create the table, insert entries, delete/update one and check that the FK has been updated?

also help me with commitlint lol

@EllaFoxo
Copy link

Yeah, I wouldn't imagine it has to be anything too complex. Try and force an update/delete and check for the intended result.

Re commit lint, it's expecting this one's message to be formatted correctly:
fix OnUpdate default string

You can do a rebase-edit to change the message - fix: change OnUpdate default string should be valid

@antipopp
Copy link
Contributor Author

I've added the tests, tell me if there is anything else to fix

@EllaFoxo
Copy link

EllaFoxo commented May 18, 2022

LGTM, I'm not a core contributor however so it'll have to wait for someone on the Uptrace team here to verify

@vmihailenco
Copy link
Member

@antipopp thanks for the PR and @tinyfluffs thanks for reviewing 👍

@vmihailenco vmihailenco merged commit a327b2a into uptrace:master Jun 8, 2022
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.

3 participants