SQLStore: Fix PostgreSQL failure to create organisation for first time #21648
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
papagian
requested review from
a team and
bergquist
and removed request for
a team
kylebrandt
reviewed
Jan 21, 2020
|
|
||
| // sync primary key sequence of org table | ||
| if _, err := sess.Exec("SELECT setval('org_id_seq', (SELECT max(id)+1 FROM org));"); err != nil { | ||
| return fmt.Errorf("Failed to sync primary key for org table") |
There was a problem hiding this comment.
This seems to follow the error pattern in the file, but I wonder:
- Why not return the error if an error?
- Regardless of what is done with the error, should the contents of sess.Exec's err be wrapped into the message so the reason is logged?
There was a problem hiding this comment.
good points, I will fix it!
There was a problem hiding this comment.
Yeah, wrapping the error sounds like the thing to do for me as well :)
aknuds1
requested changes
Jan 21, 2020
|
|
||
| // sync primary key sequence of org table | ||
| if _, err := sess.Exec("SELECT setval('org_id_seq', (SELECT max(id)+1 FROM org));"); err != nil { | ||
| return fmt.Errorf("Failed to sync primary key for org table") |
There was a problem hiding this comment.
Yeah, wrapping the error sounds like the thing to do for me as well :)
aknuds1
requested changes
Jan 22, 2020
Co-Authored-By: Arve Knudsen <arve.knudsen@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Grafana explicitly sets the org id when creates the main organisation. However, this does not work nicely with PostgresSQL because it seems that the primary key sequence is only evaluated when the id field is not set. This fix adds a hook for syncing the sequence for the org table after inserts.
Which issue(s) this PR fixes:
Fixes #18628
Special notes for your reviewer: