Merge branch '229914-remove-constraint-on-audit-event-type' into 'master'

Remove not-null on type column in audit_events

See merge request gitlab-org/gitlab!39192

changelogs/unreleased/229914-remove-constraint-on-audit-event-type.yml

+---
+title: Remove not-null constraint on type column in audit_events
+merge_request: 39192
+author:
+type: other

db/migrate/20200811055018_remove_not_null_constraint_on_type_from_audit_events.rb

+# frozen_string_literal: true
+
+class RemoveNotNullConstraintOnTypeFromAuditEvents < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  # rubocop:disable Migration/WithLockRetriesDisallowedMethod
+  # To avoid deadlock on audit_event and audit_event_part... since there is a trigger to insert record from audit_events
+  # to audit_events_part..., we need to ensure each ALTER TABLE command run in its own transaction.
+  def up
+    with_lock_retries do
+      change_column_null :audit_events_part_5fc467ac26, :type, true
+    end
+
+    with_lock_retries do
+      change_column_null :audit_events, :type, true
+    end
+  end
+  # rubocop:enable Migration/WithLockRetriesDisallowedMethod
+
+  def down
+    # no-op -- null values might be added after this constraint is removed.
+  end
+end

db/schema_migrations/20200811055018

+b7477fbcba166d848e3b1bd7f4a184f50f3b2fafe80bfcf44fd6f4f9979ffcb7
\ No newline at end of file

db/structure.sql

@@ -68,7 +68,7 @@ COMMENT ON FUNCTION public.table_sync_function_2be879775d() IS 'Partitioning mig
 CREATE TABLE public.audit_events_part_5fc467ac26 (
     id bigint NOT NULL,
     author_id integer NOT NULL,
-    type character varying NOT NULL,
+    type character varying,
     entity_id integer NOT NULL,
     entity_type character varying NOT NULL,
     details text,
@@ -9466,7 +9466,7 @@ CREATE TABLE public.ar_internal_metadata (
 CREATE TABLE public.audit_events (
     id integer NOT NULL,
     author_id integer NOT NULL,
-    type character varying NOT NULL,
+    type character varying,
     entity_id integer NOT NULL,
     entity_type character varying NOT NULL,
     details text,

spec/models/audit_event_partitioned_spec.rb

@@ -10,6 +10,13 @@ RSpec.describe AuditEventPartitioned do
     expect(partitioned_table.column_names).to match_array(source_table.column_names)
   end
 
+  it 'has the same null constraints as the source table' do
+    constraints_from_source_table = null_constraints(source_table)
+    constraints_from_partitioned_table = null_constraints(partitioned_table)
+
+    expect(constraints_from_partitioned_table.to_a).to match_array(constraints_from_source_table.to_a)
+  end
+
   it 'inserts the same record as the one in the source table', :aggregate_failures do
     expect { create(:audit_event) }.to change { partitioned_table.count }.by(1)
 
@@ -22,4 +29,13 @@ RSpec.describe AuditEventPartitioned do
 
     expect(event_from_partitioned_table).to eq(event_from_source_table)
   end
+
+  def null_constraints(table)
+    table.connection.select_all(<<~SQL)
+      SELECT c.column_name, c.is_nullable
+      FROM information_schema.columns c
+      WHERE c.table_name = '#{table.table_name}'
+      AND c.column_name != 'created_at'
+    SQL
+  end
 end