Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-- No-op: 000004 is a corrective convergence step. Both fresh installs (which
-- already had PRIMARY KEY (id) from 000001) and upgraded installs (which
-- carried the legacy composite PRIMARY KEY (id, user_id) from GORM
-- AutoMigrate) end at PRIMARY KEY (id). Once up has run, there is no way to
-- tell which path the database came from, so a symmetric down would put fresh
-- installs into a composite-PK state they never originally had. Leaving the
-- single-column PK in place on rollback is safer and the application is
-- PK-shape independent (queries do not reference the PK directly).
SELECT 1;
Comment on lines +1 to +9
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This would result in a fresh install env applying this migration on top having the wrong primary key. I think it's fine to leave as a result.

13 changes: 13 additions & 0 deletions go/core/pkg/migrations/core/000004_feedback_single_pk.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
-- Normalize the feedback primary key to (id) only.
--
-- Fresh installs from 000001 already have PRIMARY KEY (id) (BIGSERIAL).
-- Installs that upgraded from the GORM-managed schema retained the legacy
-- composite PRIMARY KEY (id, user_id) created by AutoMigrate.
--
-- Both variants use the default constraint name `feedback_pkey`, so this
-- atomic drop+add converges both paths to the single-column PK without
-- changing column types or affecting sqlc-generated code (queries do not
-- reference the PK; ListFeedback filters on user_id via idx_feedback_user_id).
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@EItanya I don't think this is necessary based on the limited usefulness of the feedback feature and the unlikelihood that this will have duplicate ids. The only reason I'm considering it is to have a clearer message for manual fixes in the unlikely event that there is an issue with this upgrade. Another option is we we could just raise it in the release notes if we want, but as before, I think this is unlikely to be an issue.

ALTER TABLE feedback
DROP CONSTRAINT feedback_pkey,
ADD CONSTRAINT feedback_pkey PRIMARY KEY (id);
Loading