Skip to content

Add check constraint to ensure star_count is non-negative

Abdul Wadood requested to merge 394810-star-count-positive-constraint into master

What does this MR do and why?

Through some unknown steps, a customer made the star_count column less than zero which prevented the updation of project settings.

Here, we fix such records and add a check constraint to ensure it never happens again.

Also, we have zero such records on gitlab.com so this is mostly for self-managed. I only expect a couple of such records maybe at most 100 records.

Query plan

https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/25726/commands/81159

Raw Query
SELECT "projects"."id"
FROM "projects"
WHERE (star_count < 0)
  AND "projects"."id" >= 654321
ORDER BY "projects"."id" ASC
LIMIT 1 OFFSET 10;

https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/25726/commands/81160

Raw Query
UPDATE "projects"
SET "star_count" = 0
WHERE (star_count < 0)
  AND "projects"."id" >= 654321
  AND "projects"."id" < 765432;

Migration output

up
bin/rails db:migrate
main: == [advisory_lock_connection] object_id: 117420, pg_backend_pid: 86043
main: == 20240123102735 UpdateNegativeStarCountsInProjects: migrating ===============
main: == 20240123102735 UpdateNegativeStarCountsInProjects: migrated (0.0310s) ======

main: == [advisory_lock_connection] object_id: 117420, pg_backend_pid: 86043
ci: == [advisory_lock_connection] object_id: 117700, pg_backend_pid: 86045
ci: == 20240123102735 UpdateNegativeStarCountsInProjects: migrating ===============
ci: -- The migration is skipped since it modifies the schemas: [:gitlab_main].
ci: -- This database can only apply migrations in one of the following schemas: [:gitlab_ci, :gitlab_internal, :gitlab_shared].
ci: == 20240123102735 UpdateNegativeStarCountsInProjects: migrated (0.0106s) ======

ci: == [advisory_lock_connection] object_id: 117700, pg_backend_pid: 86045
main: == [advisory_lock_connection] object_id: 117860, pg_backend_pid: 86048
main: == 20240123102745 AddStarCountPositiveConstraintToProjects: migrating =========
main: -- transaction_open?(nil)
main:    -> 0.0000s
main: -- transaction_open?(nil)
main:    -> 0.0000s
main: -- execute("ALTER TABLE projects\nADD CONSTRAINT projects_star_count_positive\nCHECK ( star_count >= 0 )\nNOT VALID;\n")
main:    -> 0.0017s
main: == 20240123102745 AddStarCountPositiveConstraintToProjects: migrated (0.0361s)

main: == [advisory_lock_connection] object_id: 117860, pg_backend_pid: 86048
ci: == [advisory_lock_connection] object_id: 118080, pg_backend_pid: 86051
ci: == 20240123102745 AddStarCountPositiveConstraintToProjects: migrating =========
ci: -- transaction_open?(nil)
ci:    -> 0.0000s
ci: -- transaction_open?(nil)
ci:    -> 0.0000s
ci: -- execute("ALTER TABLE projects\nADD CONSTRAINT projects_star_count_positive\nCHECK ( star_count >= 0 )\nNOT VALID;\n")
ci:    -> 0.0017s
ci: == 20240123102745 AddStarCountPositiveConstraintToProjects: migrated (0.0242s)

ci: == [advisory_lock_connection] object_id: 118080, pg_backend_pid: 86051
down
for v in 20240123102735 20240123102745; do bin/rails db:migrate:down:ci VERSION=$v && bin/rails db:migrate:down:main VERSION=$v; done
ci: == [advisory_lock_connection] object_id: 116900, pg_backend_pid: 88033
ci: == 20240123102735 UpdateNegativeStarCountsInProjects: reverting ===============
ci: -- The migration is skipped since it modifies the schemas: [:gitlab_main].
ci: -- This database can only apply migrations in one of the following schemas: [:gitlab_ci, :gitlab_internal, :gitlab_shared].
ci: == 20240123102735 UpdateNegativeStarCountsInProjects: reverted (0.0105s) ======

ci: == [advisory_lock_connection] object_id: 116900, pg_backend_pid: 88033
main: == [advisory_lock_connection] object_id: 116900, pg_backend_pid: 88429
main: == 20240123102735 UpdateNegativeStarCountsInProjects: reverting ===============
main: == 20240123102735 UpdateNegativeStarCountsInProjects: reverted (0.0101s) ======

main: == [advisory_lock_connection] object_id: 116900, pg_backend_pid: 88429
ci: == [advisory_lock_connection] object_id: 116900, pg_backend_pid: 88849
ci: == 20240123102745 AddStarCountPositiveConstraintToProjects: reverting =========
ci: -- transaction_open?(nil)
ci:    -> 0.0000s
ci: -- transaction_open?(nil)
ci:    -> 0.0000s
ci: -- execute("            ALTER TABLE projects\n            DROP CONSTRAINT IF EXISTS projects_star_count_positive\n")
ci:    -> 0.0015s
ci: == 20240123102745 AddStarCountPositiveConstraintToProjects: reverted (0.1153s)

ci: == [advisory_lock_connection] object_id: 116900, pg_backend_pid: 88849
main: == [advisory_lock_connection] object_id: 116960, pg_backend_pid: 89349
main: == 20240123102745 AddStarCountPositiveConstraintToProjects: reverting =========
main: -- transaction_open?(nil)
main:    -> 0.0000s
main: -- transaction_open?(nil)
main:    -> 0.0000s
main: -- execute("            ALTER TABLE projects\n            DROP CONSTRAINT IF EXISTS projects_star_count_positive\n")
main:    -> 0.0013s
main: == 20240123102745 AddStarCountPositiveConstraintToProjects: reverted (0.0440s)

main: == [advisory_lock_connection] object_id: 116960, pg_backend_pid: 89349

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

How to set up and validate locally

Run the migration and try to set star_count to non-negative value in rails console and it should fail:

Project.where(id: 1..100).update_all(star_count: -1)
  Project Update All (2.4ms)  UPDATE "projects" SET "star_count" = -1 WHERE "projects"."id" BETWEEN 1 AND 100
ActiveRecord::StatementInvalid: PG::CheckViolation: ERROR:  new row for relation "projects" violates check constraint "projects_star_count_positive"

Related to #394810 (closed)

Edited by Abdul Wadood

Merge request reports