Add migration helpers for managing check constraints and limits on text columns (!29181) · Merge requests · GitLab.org / GitLab

Yannis Roussos requested to merge 30453-add-migration-helpers-for-limiting-text-columns into master Apr 08, 2020

What does this MR do?

This MR adds Migration Helpers for managing check constraints.

With our switch to using structure.sql to capture the current database schema, this MR is our first implementation of a pure PostgreSQL feature that could not be supported while we were using schema.rb.

We are adding support for text limits as our first step, but the additional (generic) helpers that are introduced will form the base for implementing other features that will result in significant performance and reliability improvements on the database side. Examples include the delayed validation of SET NOT NULL constraints (#38358 (closed)) or the ability to add various other constraints on columns and delay their validation.

Summary of updates:

Adds generic migration helpers for managing check constraints

Adding, removing, validating, checking if a constraint exists and generating a unique constraint name are supported
Adds migration helpers for adding/removing limits to text columns
Adds rspec tests for all the newly introduced migration helpers.

In order to keep the MR scope manageable, this is the first of three MRs required for successfully completing #30453 (closed)

Implement migration helpers for adding CHECK constraints for limiting the size of text columns (current MR)
Enforce placing a limit on text columns with rubocop (followup MR that will check that the add_text_limit helper is always used when a text column is added - !29304 (merged))
Disallow using varchar(N), i.e. string columns in favor of using text always (Also part of !29304 (merged))
Update Migration Documentation with the new options (followup MR)

Quick Overview of the supported migration helpers and Demo of the functionality provided

The main focus is to support any generic check constraint, but also provide easy to use migration helpers for specific use cases.

The generic migration helpers are intended as the basis for the specialised helpers, but also provide a way to add any constraint a migration author may want to add.

Adding Limits to Text columns

The most simple use case is adding a limit to a text column after creating a new table.

In this case, there is no need to delay the validation.

That's how a migration using the new add_text_limit and remove_text_limit will look like:

class TestTextLimitMigrationHelpers < ActiveRecord::Migration[6.0]
  include Gitlab::Database::MigrationHelpers
  DOWNTIME = false

  disable_ddl_transaction!

  def up
    create_table :test_text_limits, id: false do |t|
      t.integer :test_id, null: false
      t.text :name
    end

    add_text_limit :test_text_limits, :name, 5
  end

  def down
    remove_text_limit :test_text_limits, :name
    # On purpose not removing the table to showcase the constraint removal
    # drop_table :test_text_limits
  end
end

Running the migration properly adds the new constraint:

$ bundle exec rake db:migrate
== 20200408105709 TestTextLimitMigrationHelpers: migrating ====================
-- create_table(:test_text_limits, {:id=>false})
   -> 0.0039s
-- transaction_open?()
   -> 0.0000s
-- execute("ALTER TABLE test_text_limits\nADD CONSTRAINT check_ec076ae08c\nCHECK ( char_length(name) <= 5 )\nNOT VALID;\n")
   -> 0.0006s
-- execute("SET statement_timeout TO 0")
   -> 0.0001s
-- execute("ALTER TABLE test_text_limits VALIDATE CONSTRAINT check_ec076ae08c;")
   -> 0.0003s
-- execute("RESET ALL")
   -> 0.0001s
== 20200408105709 TestTextLimitMigrationHelpers: migrated (0.0063s) ===========

$ git diff db/structure.sql

+CREATE TABLE public.test_text_limits (
+    test_id integer NOT NULL,
+    name text,
+    CONSTRAINT check_ec076ae08c CHECK ((char_length(name) <= 5))
+);

$ gdk psql
gitlabhq_development=# \d+ test_text_limits
                              Table "public.test_text_limits"
 Column  |  Type   | Collation | Nullable | Default | Storage  | Stats target | Description
---------+---------+-----------+----------+---------+----------+--------------+-------------
 test_id | integer |           | not null |         | plain    |              |
 name    | text    |           |          |         | extended |              |
Check constraints:
    "check_ec076ae08c" CHECK (char_length(name) <= 5)

gitlabhq_development=# INSERT INTO test_text_limits VALUES (1, 'john');
INSERT 0 1

gitlabhq_development=# INSERT INTO test_text_limits VALUES (2, 'yannis');
ERROR:  new row for relation "test_text_limits" violates check constraint "check_ec076ae08c"
DETAIL:  Failing row contains (2, yannis).

Rolling back the migration removes the constraints (drop table not included in this migration on purpose):

$ bundle exec rake db:rollback
== 20200408105709 TestTextLimitMigrationHelpers: reverting ====================
-- execute("ALTER TABLE test_text_limits\nDROP CONSTRAINT IF EXISTS check_ec076ae08c\n")
   -> 0.0016s
== 20200408105709 TestTextLimitMigrationHelpers: reverted (0.0021s) ===========

$ git diff db/structure.sql

+CREATE TABLE public.test_text_limits (
+    test_id integer NOT NULL,
+    name text
+);

$ gdk psql
gitlabhq_development=# \d+ test_text_limits
                              Table "public.test_text_limits"
 Column  |  Type   | Collation | Nullable | Default | Storage  | Stats target | Description
---------+---------+-----------+----------+---------+----------+--------------+-------------
 test_id | integer |           | not null |         | plain    |              |
 name    | text    |           |          |         | extended |              |

gitlabhq_development=# INSERT INTO test_text_limits VALUES (1, 'yannis');
INSERT 0 1

Helpers for delayed validation

The same is true if we want to validate at a later time.

Let's assume that we want to add a limit on an existing table with heavy usage patterns. In the following example we create the table and do everything on the same migration to test and showcase the check_text_limit_exists? and validate_text_limit helpers.

In real life conditions, when developing against our production Database, the add_text_limit with validate: false will be part of a first MR, with an additional post migration to clean any inconsistent data and a final migration in the next milestone to run the validate_text_limit.

class TestTextLimitMigrationHelpers < ActiveRecord::Migration[6.0]
  include Gitlab::Database::MigrationHelpers
  DOWNTIME = false

  disable_ddl_transaction!

  def up
    create_table :test_text_limits, id: false do |t|
      t.integer :test_id, null: false
      t.text :name
    end

    add_text_limit :test_text_limits, :name, 5, validate: false

    # Not required here, just showcasing and testing the helper
    if check_text_limit_exists?(:test_text_limits, :name)
      validate_text_limit :test_text_limits, :name
    end
  end

  def down
    remove_text_limit :test_text_limits, :name
  end
end

Running the migration:

$ bundle exec rake db:migrate
== 20200408105709 TestTextLimitMigrationHelpers: migrating ====================
-- create_table(:test_text_limits, {:id=>false})
   -> 0.0030s
-- transaction_open?()
   -> 0.0000s
-- execute("ALTER TABLE test_text_limits\nADD CONSTRAINT check_ec076ae08c\nCHECK ( char_length(name) <= 5 )\nNOT VALID;\n")
   -> 0.0008s
-- execute("SET statement_timeout TO 0")
   -> 0.0001s
-- execute("ALTER TABLE test_text_limits VALIDATE CONSTRAINT check_ec076ae08c;")
   -> 0.0004s
-- execute("RESET ALL")
   -> 0.0001s
== 20200408105709 TestTextLimitMigrationHelpers: migrated (0.0064s) ===========

$ git diff db/structure.sql

+CREATE TABLE public.test_text_limits (
+    test_id integer NOT NULL,
+    name text,
+    CONSTRAINT check_ec076ae08c CHECK ((char_length(name) <= 5))
+);

$ gdk psql
gitlabhq_development=# \d+ test_text_limits
                              Table "public.test_text_limits"
 Column  |  Type   | Collation | Nullable | Default | Storage  | Stats target | Description
---------+---------+-----------+----------+---------+----------+--------------+-------------
 test_id | integer |           | not null |         | plain    |              |
 name    | text    |           |          |         | extended |              |
Check constraints:
    "check_ec076ae08c" CHECK (char_length(name) <= 5)

gitlabhq_development=# INSERT INTO test_text_limits VALUES (1, 'yannis');
ERROR:  new row for relation "test_text_limits" violates check constraint "check_ec076ae08c"
DETAIL:  Failing row contains (1, yannis).

Rolling back:

$ bundle exec rake db:rollback
== 20200408105709 TestTextLimitMigrationHelpers: reverting ====================
-- execute("ALTER TABLE test_text_limits\nDROP CONSTRAINT IF EXISTS check_ec076ae08c\n")
   -> 0.0015s
== 20200408105709 TestTextLimitMigrationHelpers: reverted (0.0016s) ===========

$ git diff db/structure.sql

+CREATE TABLE public.test_text_limits (
+    test_id integer NOT NULL,
+    name text
+);

$ gdk psql
gitlabhq_development=# \d+ test_text_limits
                              Table "public.test_text_limits"
 Column  |  Type   | Collation | Nullable | Default | Storage  | Stats target | Description
---------+---------+-----------+----------+---------+----------+--------------+-------------
 test_id | integer |           | not null |         | plain    |              |
 name    | text    |           |          |         | extended |              |

Generic migration helpers for adding any constraint

In the following example we showcase how the base, generic migration helpers work and how they could be used for covering edge cases with no need for specialised helpers.

We add a check constraint that uses like and use the new migration helpers to add a NOT NULL in a non blocking manner:

class TestTextLimitMigrationHelpers < ActiveRecord::Migration[6.0]
  include Gitlab::Database::MigrationHelpers
  DOWNTIME = false

  disable_ddl_transaction!

  def up
    create_table :test_text_limits, id: false do |t|
      t.integer :test_id, null: false
      t.text :name
    end

    # Add a constraint without validating
    add_check_constraint :test_text_limits, "name like 'yannis%'", con_name, validate: false

    if check_constraint_exists?(:test_text_limits, con_name)
      validate_check_constraint :test_text_limits, con_name
    end

    # Add a NOT NULL constraint and validate without blocking
    add_check_constraint :test_text_limits, 'name IS NOT NULL', 'check_name_not_null'
  end

  def down
    remove_check_constraint :test_text_limits, 'check_name_not_null'
    remove_check_constraint :test_text_limits, con_name
    # drop_table :test_text_limits
  end

  private

  def con_name
    check_constraint_name(:test_text_limits, :name, type: 'only_yannis_allowed')
  end
end

Running the migration:

$ bundle exec rake db:migrate
== 20200408144836 TestTextLimitMigrationHelpers: migrating ====================
-- create_table(:test_text_limits, {:id=>false})
   -> 0.0039s
-- transaction_open?()
   -> 0.0000s
-- execute("ALTER TABLE test_text_limits\nADD CONSTRAINT check_1256fd639e\nCHECK ( name like 'yannis%' )\nNOT VALID;\n")
   -> 0.0005s
-- execute("SET statement_timeout TO 0")
   -> 0.0001s
-- execute("ALTER TABLE test_text_limits VALIDATE CONSTRAINT check_1256fd639e;")
   -> 0.0004s
-- execute("RESET ALL")
   -> 0.0002s
-- transaction_open?()
   -> 0.0000s
-- execute("ALTER TABLE test_text_limits\nADD CONSTRAINT check_name_not_null\nCHECK ( name IS NOT NULL )\nNOT VALID;\n")
   -> 0.0002s
-- execute("SET statement_timeout TO 0")
   -> 0.0001s
-- execute("ALTER TABLE test_text_limits VALIDATE CONSTRAINT check_name_not_null;")
   -> 0.0004s
-- execute("RESET ALL")
   -> 0.0001s
== 20200408144836 TestTextLimitMigrationHelpers: migrated (0.0115s) ===========

$ git diff db/structure.sql

+CREATE TABLE public.test_text_limits (
+    test_id integer NOT NULL,
+    name text,
+    CONSTRAINT check_1256fd639e CHECK ((name ~~ 'yannis%'::text)),
+    CONSTRAINT check_name_not_null CHECK ((name IS NOT NULL))
+);

$ gdk psql
gitlabhq_development=# \d+ test_text_limits
                              Table "public.test_text_limits"
 Column  |  Type   | Collation | Nullable | Default | Storage  | Stats target | Description
---------+---------+-----------+----------+---------+----------+--------------+-------------
 test_id | integer |           | not null |         | plain    |              |
 name    | text    |           |          |         | extended |              |
Check constraints:
    "check_1256fd639e" CHECK (name ~~ 'yannis%'::text)
    "check_name_not_null" CHECK (name IS NOT NULL)

gitlabhq_development=# INSERT INTO test_text_limits VALUES (1, 'yannis roussos');
INSERT 0 1

gitlabhq_development=# INSERT INTO test_text_limits VALUES (2, 'johny');
ERROR:  new row for relation "test_text_limits" violates check constraint "check_1256fd639e"
DETAIL:  Failing row contains (2, johny).

gitlabhq_development=# INSERT INTO test_text_limits VALUES (2, null);
ERROR:  new row for relation "test_text_limits" violates check constraint "check_name_not_null"
DETAIL:  Failing row contains (2, null).

Rolling back:

$ bundle exec rake db:rollback
== 20200408144836 TestTextLimitMigrationHelpers: reverting ====================
-- execute("ALTER TABLE test_text_limits\nDROP CONSTRAINT IF EXISTS check_name_not_null\n")
   -> 0.0008s
-- execute("ALTER TABLE test_text_limits\nDROP CONSTRAINT IF EXISTS check_1256fd639e\n")
   -> 0.0003s
== 20200408144836 TestTextLimitMigrationHelpers: reverted (0.0095s) ===========

$ git diff db/structure.sql

+CREATE TABLE public.test_text_limits (
+    test_id integer NOT NULL,
+    name text
+);

$ gdk psql
gitlabhq_development=# \d+ test_text_limits
                              Table "public.test_text_limits"
 Column  |  Type   | Collation | Nullable | Default | Storage  | Stats target | Description
---------+---------+-----------+----------+---------+----------+--------------+-------------
 test_id | integer |           | not null |         | plain    |              |
 name    | text    |           |          |         | extended |              |

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
[-] Tested in all supported browsers
[-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

[-] Label as security and @ mention @gitlab-com/gl-security/appsec
[-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
[-] Security reports checked/validated by a reviewer from the AppSec team

Edited May 31, 2022 by 🤖 GitLab Bot 🤖

Add migration helpers for managing check constraints and limits on text columns