-
Notifications
You must be signed in to change notification settings - Fork 2
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
[MM-60328] include proper search_path check to post-migrate #26
Conversation
This "bug" affected me, and after hours of search I found the solution.
The
But it is wrong. To make it work, I had to set by hand the it as
After that, this is what the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my understanding, https://mattermost.atlassian.net/browse/MM-60328 was about restoring the schema_search path in case the command failed to run successfully. But this PR just adds a post-migrate check correct?
Can we do something so that the search_path gets restored correctly in case of failure? If not, clearly mention that "command has failed, search_path could not be restored, please reset it manually" or something like that?
@agnivade Since |
internal/store/postgres.go
Outdated
if len(schemas) == 0 { | ||
return fmt.Errorf("no value available for search_path") | ||
} else if _, ok := slices.BinarySearch(schemas, schema); !ok { | ||
} else if _, ok := slices.BinarySearch(schemas, fmt.Sprintf("%q, %s", "$user", schema)); !ok { | ||
logger.Printf("could not find the default schema %q in search_path, consider setting it from the postgresql console\n", schema) | ||
err := db.ExecQuery(ctx, fmt.Sprintf("SELECT pg_catalog.set_config('search_path', '\"$user\", %s', false)", schema)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think now we can refactor both these strings into a single declaration above. Because we can use %q
to avoid the quote escaping.
So to clarify: this schema check is only needed to run the index creation? I thought this was there as a sanity check to run Mattermost later? |
@agnivade nope, I mean this command will run (at least we'll advise that) after whenever a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved offline
Summary
After every migration, we should advise running
migration-assist post-migrate
command to determine the health of the DB in terms of schema and it's ownership.Also make index creation optional. Will add documentation accordingly.
Ticket Link
https://mattermost.atlassian.net/browse/MM-60328