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

add primary key to github_event table #35

Merged
merged 1 commit into from
Jul 9, 2021
Merged

Conversation

DanCech
Copy link
Contributor

@DanCech DanCech commented Jul 8, 2021

fixes #34

I ended up just adding a simple version of the raw sql migration here since I couldn't import a newer version of grafana/grafana and the syntax is the same for both mysql and postgres

@DanCech DanCech requested review from radiohead and marefr July 8, 2021 22:54
Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this!

I think the historical reason not having primary key for this might be that event id might not be unique/missing or something like that - it's coming from GitHub.

{Name: "id", Type: migrator.DB_BigInt},

Parsed here and could in theory be zero:

id, err := strconv.ParseInt(ge.ID, 10, 0)

An alternative could be to only add an index, but lets give this a go and see what happens

@DanCech
Copy link
Contributor Author

DanCech commented Jul 9, 2021

Since the code already issues a DELETE by id before the insert we couldn't have duplicates anyway.

if _, err := session.Exec("DELETE FROM github_event WHERE ID = ? ", event.ID); err != nil {
return err
}
if _, err := session.Insert(event); err != nil {
return err
}

@DanCech DanCech merged commit 4f0d0f2 into master Jul 9, 2021
@DanCech DanCech deleted the github_event-primary-key branch July 9, 2021 13:49
@marefr
Copy link
Member

marefr commented Jul 9, 2021

Yeah good point 😄 👍

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.

github_event table has no indexes
2 participants