Commit f9285d82 authored by Albert Salim's avatar Albert Salim Committed by Mayra Cabrera

Check for enums not using smallint

parent d2847f16
......@@ -118,6 +118,7 @@ description: 'Learn how to contribute to GitLab.'
- [Query Count Limits](query_count_limits.md)
- [Database helper modules](database_helpers.md)
- [Code comments](code_comments.md)
- [Creating enums](creating_enums.md)
### Case studies
......
# Creating enums
When creating a new enum, it should use the database type `SMALLINT`.
The `SMALLINT` type size is 2 bytes, which is sufficient for an enum.
This would help to save space in the database.
To use this type, add `limit: 2` to the migration that creates the column.
Example:
```rb
def change
add_column :ci_job_artifacts, :file_format, :integer, limit: 2
end
```
......@@ -29,11 +29,20 @@ module EE
vulnerability_scanners: %w[external_id],
web_hooks: %w[group_id]
}.with_indifferent_access.freeze
IGNORED_LIMIT_ENUMS = {
'SoftwareLicensePolicy' => %w[approval_status],
'User' => %w[group_view]
}.freeze
end
def ignored_fk_columns(column)
super + EE_IGNORED_FK_COLUMNS.fetch(column, [])
end
def ignored_limit_enums(model)
super + IGNORED_LIMIT_ENUMS.fetch(model, [])
end
end
end
end
......@@ -120,9 +120,55 @@ describe 'Database schema' do
end
end
# These pre-existing enums have limits > 2 bytes
IGNORED_LIMIT_ENUMS = {
'Analytics::CycleAnalytics::GroupStage' => %w[start_event_identifier end_event_identifier],
'Analytics::CycleAnalytics::ProjectStage' => %w[start_event_identifier end_event_identifier],
'Ci::Bridge' => %w[failure_reason],
'Ci::Build' => %w[failure_reason],
'Ci::BuildMetadata' => %w[timeout_source],
'Ci::BuildTraceChunk' => %w[data_store],
'Ci::JobArtifact' => %w[file_type],
'Ci::Pipeline' => %w[source config_source failure_reason],
'Ci::Runner' => %w[access_level],
'Ci::Stage' => %w[status],
'Clusters::Applications::Ingress' => %w[ingress_type],
'Clusters::Cluster' => %w[platform_type provider_type],
'CommitStatus' => %w[failure_reason],
'GenericCommitStatus' => %w[failure_reason],
'Gitlab::DatabaseImporters::CommonMetrics::PrometheusMetric' => %w[group],
'InternalId' => %w[usage],
'List' => %w[list_type],
'NotificationSetting' => %w[level],
'Project' => %w[auto_cancel_pending_pipelines],
'ProjectAutoDevops' => %w[deploy_strategy],
'PrometheusMetric' => %w[group],
'ResourceLabelEvent' => %w[action],
'User' => %w[layout dashboard project_view],
'UserCallout' => %w[feature_name],
'PrometheusAlert' => %w[operator]
}.freeze
context 'for enums' do
ApplicationRecord.descendants.each do |model|
describe model do
let(:ignored_enums) { ignored_limit_enums(model.name) }
let(:enums) { model.defined_enums.keys - ignored_enums }
it 'uses smallint for enums' do
expect(model).to use_smallint_for_enums(enums)
end
end
end
end
private
def ignored_fk_columns(column)
IGNORED_FK_COLUMNS.fetch(column, [])
end
def ignored_limit_enums(model)
IGNORED_LIMIT_ENUMS.fetch(model, [])
end
end
# frozen_string_literal: true
EXPECTED_SMALLINT_LIMIT = 2
RSpec::Matchers.define :use_smallint_for_enums do |enums|
match do |actual|
@failing_enums = enums.select do |enum|
enum_type = actual.type_for_attribute(enum)
actual_limit = enum_type.send(:subtype).limit
actual_limit != EXPECTED_SMALLINT_LIMIT
end
@failing_enums.empty?
end
failure_message do
<<~FAILURE_MESSAGE
Expected #{actual.name} enums: #{failing_enums.join(', ')} to use the smallint type.
The smallint type is 2 bytes which is more than sufficient for an enum.
Using the smallint type would help us save space in the database.
To fix this, please add `limit: 2` in the migration file, for example:
def change
add_column :ci_job_artifacts, :file_format, :integer, limit: 2
end
FAILURE_MESSAGE
end
def failing_enums
@failing_enums ||= []
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment