-
Notifications
You must be signed in to change notification settings - Fork 182
Fix panic when build not found because concurrent access #1103
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
Conversation
|
can we still have the option to do migrate/down? I use it quite extensively for development |
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.
Bug: Role and Function Creation Fails on Existing Entries
The setupAuthSchema function unconditionally attempts to create the authenticated role and auth.uid() function. Since the existence check only applies to the auth.users table, the migration can fail if the role or function already exist from a previous attempt or an inconsistent state.
packages/db/scripts/migrator.go#L124-L147
infra/packages/db/scripts/migrator.go
Lines 124 to 147 in 344fd78
| // Create authenticated user | |
| _, err = db.Exec("CREATE ROLE authenticated;") | |
| if err != nil { | |
| return err | |
| } | |
| // Create users table | |
| _, err = db.Exec( | |
| `CREATE TABLE IF NOT EXISTS auth.users (id uuid NOT NULL DEFAULT gen_random_uuid(),email text NOT NULL, PRIMARY KEY (id));`) | |
| if err != nil { | |
| return err | |
| } | |
| // Create function to generate a random uuid | |
| _, err = db.Exec( | |
| `CREATE FUNCTION auth.uid() RETURNS uuid AS $func$ | |
| BEGIN | |
| RETURN gen_random_uuid(); | |
| END; | |
| $func$ LANGUAGE plpgsql;`) | |
| if err != nil { | |
| return err | |
| } | |
The previous query was not run in a transaction, and when some builds were deleted/state changes were made later, filtering failed when accessing the array without builds.
Decided to use sqlc, so everything is now sql-native and the query runs in one transaction. Also, removing the additional part that was still using the deprecated ORM for accessing the database.