Skip to content

Fix not-null inconsistency

Andreas Brandl requested to merge ab/fix-not-null-inconsistency into master

What does this MR do?

We're reverting changes to a migration helper with !31886 (merged).

We know there is one migration that used the changed helper "in the wild" (on GitLab.com) - which is what we are correcting here.

This basically fixes the inconsistency: GitLab.com has two CHECK constraints, but in db/structure.sql we expect those to be NOT NULL constraints.

The idea here is to align systems with the inconsistency with db/structure.sql. This goes by:

  1. Making sure we have NOT NULL constraints on the column
  2. Dropping the CHECK constraints should they exists.

The #down part has been left intentionally blank as reverting this doesn't really make sense (and is not in line with what db/structure.sql expects).

This might also be the case for dev environments or other early-bird systems (that have the helper changed deployed already).

Migration impact

GitLab.com (and perhaps other systems)
== 20200513160930 FixNotNullCheckConstraintInconsistency: migrating ===========
-- change_column_null(:application_settings, :container_registry_vendor, false)
   -> 0.0010s
-- execute("ALTER TABLE application_settings\nDROP CONSTRAINT IF EXISTS check_b7c8e8cfa6\n")
   -> 0.0022s
-- change_column_null(:application_settings, :container_registry_version, false)
   -> 0.0007s
-- execute("ALTER TABLE application_settings\nDROP CONSTRAINT IF EXISTS check_57d804fa8a\n")
   -> 0.0008s
== 20200513160930 FixNotNullCheckConstraintInconsistency: migrated (0.0198s) ==
regular systems

For regular systems not having the inconsistency, this is going to be a no-op:

== 20200513160930 FixNotNullCheckConstraintInconsistency: migrating ===========
-- change_column_null(:application_settings, :container_registry_vendor, false)
   -> 0.0008s
-- change_column_null(:application_settings, :container_registry_version, false)
   -> 0.0003s
== 20200513160930 FixNotNullCheckConstraintInconsistency: migrated (0.0024s) ==

Does this MR meet the acceptance criteria?

Conformity

Edited by 🤖 GitLab Bot 🤖

Merge request reports