## Problem When an integer column is converted to bigint, a NOT NULL guarantee that is expressed as a check constraint is silently lost. ## Why it happens `Gitlab::Database::Migrations::Conversions::BigintConverter` creates the temporary `*_convert_to_bigint` column and mirrors the source column's nullability with `null: nullable?(column)`. `nullable?` returns true when the column is nullable at the column level. It does not look at check constraints. So for a column that is nullable but has `CHECK ((col IS NOT NULL))`, the temporary bigint column is created nullable and with no equivalent check. In PostgreSQL a check constraint is bound to the physical column, not to the column name. After the swap renames the columns, the original constraint still guards the retired integer column and reads `col_convert_to_bigint IS NOT NULL`. The cleanup step drops the retired column, and the constraint goes with it. The NOT NULL guarantee is then gone for good. Between swap and cleanup the guarantee survives only by accident. The sync trigger writes both columns, so a NULL insert still trips the old constraint. Nothing catches this: - The documented swap process in `doc/development/database/avoiding_downtime_in_migrations.md` lists what to swap: columns, trigger function, defaults, primary key constraint, indexes, foreign keys. Check constraints are not mentioned. - No swap migration in `db/post_migrate` handles a check constraint. - No spec or RuboCop rule asserts that a sharding key column is NOT NULL, so the loss would not fail CI. ## Scope Measured against current master. `db/integer_ids_not_yet_initialized_to_bigint.yml` lists 239 tables and 541 columns that are still integer on existing instances. Of those, 27 columns are nullable at the column level and have a `CHECK ((col IS NOT NULL))` constraint. All 27 are exposed. About 17 of the 27 look like Cells sharding keys (project_id, namespace_id, group_id, target_project_id). The rest are ordinary NOT NULL guarantees. The 27 columns: - `analytics_devops_adoption_segments.namespace_id` - `analytics_devops_adoption_snapshots.namespace_id` - `board_group_recent_visits.board_id` - `board_group_recent_visits.group_id` - `board_group_recent_visits.user_id` - `board_project_recent_visits.board_id` - `board_project_recent_visits.project_id` - `board_project_recent_visits.user_id` - `ci_pipeline_schedules.project_id` - `ci_runner_projects.project_id` - `ci_sources_pipelines.project_id` - `ci_triggers.project_id` - `epics.issue_id` - `gitlab_subscription_histories.namespace_id` - `gitlab_subscriptions.namespace_id` - `gpg_keys.user_id` - `gpg_signatures.project_id` - `identities.user_id` - `merge_request_metrics.target_project_id` - `merge_requests_compliance_violations.target_project_id` - `pages_domains.project_id` - `path_locks.project_id` - `releases.project_id` - `remote_mirrors.project_id` - `slack_integrations.integration_id` - `spam_logs.user_id` - `subscriptions.user_id` ## How this was measured 1. Parse every `CREATE TABLE` body in `db/structure.sql` at current master. 2. For each table and column in `db/integer_ids_not_yet_initialized_to_bigint.yml`, look for `CHECK ((<column> IS NOT NULL))` in that table's body. 3. Check the column definition line for a literal `NOT NULL`. ## Caveats - 17 tables in the yml are not plain `CREATE TABLE` entries in `db/structure.sql`, mostly partitioned ones, and were skipped. The real count may be higher. - The yml mixes conversions that have not started with conversions already in flight. The schema alone does not say which temporary columns already exist in production. - Tables already swapped and cleaned up were not checked. If any of those had a check constraint, the schema no longer shows any trace, so only the migration history would tell. ## Proposed options 1. At init time, have `BigintConverter` detect `CHECK ((col IS NOT NULL))` on a converted column and create the matching check on the temporary column. The swap then inherits it and no later work is needed. This only helps conversions that have not been initialized yet. 2. Have `BigintConverter` refuse to initialize such a column and tell the author to handle the constraint, so the problem is loud rather than silent. 3. Add a check to the swap tooling or a spec, so a swap that leaves a check constraint on a retired column fails. 4. Document the case in the swap section of the migration docs. ### Smaller, separate item The docs snippet for step 1 of the swap section shows `restrict_gitlab_migration gitlab_schema: :gitlab_ci`. That line belongs to the `ensure_backfill_conversion_of_integer_to_bigint` migration, because that one really is DML. Copying it into a pure DDL swap migration makes it fail: the query analyzer then runs in DML mode and rejects DDL. A short note in the docs would prevent that. ## Related The swap for `ci_sources_pipelines` hit this: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/251341 A database reviewer caught it during review. That MR works around it in the swap migration itself. It renames the old check to a temporary name, re-adds the check on the new bigint column as NOT VALID inside the same transaction (catalog only), and validates the existing rows in a separate follow-up migration.