Skip to content

Commit

Permalink
unindexed_foreign_keys: consider column type for foreign key candidates
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Jan 5, 2025
1 parent 88c85d0 commit 58b6b70
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 8 deletions.
7 changes: 7 additions & 0 deletions lib/active_record_doctor/detectors/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,13 @@ def each_column(table_name, only: nil, except: [])
end
end

def looks_like_foreign_key?(column)
type = column.type.to_s

column.name.end_with?("_id") &&
(type == "integer" || type.include?("uuid"))
end

def each_foreign_key(table_name)
log("Iterating over foreign keys on #{table_name}") do
connection.foreign_keys(table_name).each do |foreign_key|
Expand Down
7 changes: 0 additions & 7 deletions lib/active_record_doctor/detectors/missing_foreign_keys.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,6 @@ def detect
end
end

def looks_like_foreign_key?(column)
type = column.type.to_s

column.name.end_with?("_id") &&
(type == "integer" || type.include?("uuid"))
end

def foreign_key?(table, column)
connection.foreign_keys(table).any? do |foreign_key|
foreign_key.options[:column] == column.name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def message(table:, columns:)
def detect
each_table(except: config(:ignore_tables)) do |table|
each_column(table, except: config(:ignore_columns)) do |column|
next unless named_like_foreign_key?(column) || foreign_key?(table, column)
next unless looks_like_foreign_key?(column) || foreign_key?(table, column)
next if indexed?(table, column)
next if indexed_as_polymorphic?(table, column)
next if connection.primary_key(table) == column.name
Expand Down
20 changes: 20 additions & 0 deletions test/active_record_doctor/detectors/unindexed_foreign_keys_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,26 @@ def test_indexed_polymorphic_foreign_key_is_not_reported
refute_problems
end

def test_non_integer_unindexed_foreign_key_is_not_reported
Context.create_table(:users) do |t|
t.string :external_id
end

refute_problems
end

def test_uuid_unindexed_foreign_key_is_reported
require_uuid_column_type!

Context.create_table(:users) do |t|
t.uuid :company_id
end

assert_problems(<<~OUTPUT)
add an index on users(company_id) - foreign keys are often used in database lookups and should be indexed for performance reasons
OUTPUT
end

def test_indexed_foreign_key_is_not_reported
Context.create_table(:companies)
Context.create_table(:users) do |t|
Expand Down

0 comments on commit 58b6b70

Please sign in to comment.