Refactor/Fix missing foreign key index detection + missing FK constraints heuristics and update tests

lib/ruby-pg-extras.rb

@@ -7,6 +7,7 @@
 require "ruby_pg_extras/diagnose_data"
 require "ruby_pg_extras/diagnose_print"
 require "ruby_pg_extras/detect_fk_column"
+require "ruby_pg_extras/ignore_list"
 require "ruby_pg_extras/missing_fk_indexes"
 require "ruby_pg_extras/missing_fk_constraints"
 require "ruby_pg_extras/index_info"
@@ -195,7 +196,7 @@ def self.missing_fk_indexes(args: {}, in_format: :display_table)
   end
 
   def self.missing_fk_constraints(args: {}, in_format: :display_table)
-    RubyPgExtras::MissingFkConstraints.call(args[:table_name])
+    RubyPgExtras::MissingFkConstraints.call(args[:table_name], ignore_list: args[:ignore_list])
   end
 
   def self.display_result(result, title:, in_format:)

lib/ruby_pg_extras/detect_fk_column.rb

@@ -34,16 +34,25 @@ def self.call(column_name, tables)
     end
 
     def call(column_name, tables)
-      return false unless column_name =~ /_id$/
-      table_name = column_name.split("_").first
-      table_name = pluralize(table_name)
-      tables.include?(table_name)
+      # Heuristic: Rails-style foreign keys are usually named `<table_singular>_id`.
+      # We accept underscores in the prefix (e.g. `account_user_id` -> `account_users`).
+      match = /\A(?<table_singular>.+)_id\z/i.match(column_name.to_s)
+      return false unless match
+
+      table_singular = match[:table_singular]
+      return false if table_singular.empty?
+
+      tables.include?(pluralize(table_singular))
     end
 
     def pluralize(word)
-      return word if UNCOUNTABLE.include?(word.downcase)
-      return IRREGULAR[word] if IRREGULAR.key?(word)
-      return IRREGULAR.invert[word] if IRREGULAR.value?(word)
+      # Table names from Postgres are typically lowercase. Normalize before applying rules.
+      word = word.to_s.downcase
+
+      return word if UNCOUNTABLE.include?(word)
+      return IRREGULAR.fetch(word) if IRREGULAR.key?(word)
+      # If the word is already an irregular plural (e.g. "people"), keep it as-is.
+      return word if IRREGULAR.value?(word)
 
       PLURAL_RULES.reverse.each do |(rule, replacement)|
         return word.gsub(rule, replacement) if word.match?(rule)

lib/ruby_pg_extras/ignore_list.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+module RubyPgExtras
+  # Parses and matches ignore patterns like:
+  # - "*"              (ignore everything)
+  # - "posts.*"        (ignore all columns on a table)
+  # - "category_id"    (ignore this column name on all tables)
+  # - "posts.topic_id" (ignore a specific table+column)
+  class IgnoreList
+    def initialize(ignore_list)
+      @rules = normalize(ignore_list)
+    end
+
+    def ignored?(table:, column_name:)
+      @rules.any? do |rule|
+        next true if rule == "*"
+        next true if rule == "#{table}.*"
+        next true if rule == column_name
+        next true if rule == "#{table}.#{column_name}"
+        false
+      end
+    end
+
+    private
+
+    def normalize(ignore_list)
+      entries =
+        case ignore_list
+        when nil
+          []
+        when String
+          ignore_list.split(",")
+        when Array
+          ignore_list
+        else
+          raise ArgumentError, "ignore_list must be a String or Array"
+        end
+
+      entries
+        .map { |v| v.to_s.strip }
+        .reject(&:empty?)
+        .uniq
+    end
+  end
+end
+
+

lib/ruby_pg_extras/missing_fk_constraints.rb

@@ -2,42 +2,68 @@
 
 module RubyPgExtras
   class MissingFkConstraints
-    def self.call(table_name)
-      new.call(table_name)
+    # ignore_list: array (or comma-separated string) of entries like:
+    # - "posts.category_id" (ignore a specific table+column)
+    # - "category_id"       (ignore this column name for all tables)
+    # - "posts.*"           (ignore all columns on a table)
+    # - "*"                 (ignore everything)
+    def self.call(table_name, ignore_list: nil)
+      new.call(table_name, ignore_list: ignore_list)
     end
 
-    def call(table_name)
-      tables = if table_name
+    def call(table_name, ignore_list: nil)
+      ignore_list_matcher = IgnoreList.new(ignore_list)
+
+      tables =
+        if table_name
           [table_name]
         else
           all_tables
         end
 
-      schemas = query_module.table_schemas(in_format: :hash)
-      foreign_keys = query_module.foreign_keys(in_format: :hash)
+      schemas_by_table = query_module
+        .table_schemas(in_format: :hash)
+        .group_by { |row| row.fetch("table_name") }
 
-      tables.reduce([]) do |agg, table|
-        schema = schemas.select { |row| row.fetch("table_name") == table }
-        fk_columns = foreign_keys.select { |row| row.fetch("table_name") == table }
+      fk_columns_by_table = query_module
+        .foreign_keys(in_format: :hash)
+        .group_by { |row| row.fetch("table_name") }
+        .transform_values { |rows| rows.map { |row| row.fetch("column_name") } }
 
-        fk_columns = schema.filter_map do |row|
-          if DetectFkColumn.call(row.fetch("column_name"), all_tables)
-            row.fetch("column_name")
-          end
-        end
+      tables.each_with_object([]) do |table, agg|
+        schema = schemas_by_table.fetch(table, [])
+        fk_columns_for_table = fk_columns_by_table.fetch(table, [])
+        schema_column_names = schema.map { |row| row.fetch("column_name") }
+
+        candidate_fk_columns = schema.filter_map do |row|
+          column_name = row.fetch("column_name")
+
+          # Skip columns explicitly excluded via ignore list.
+          next if ignore_list_matcher.ignored?(table: table, column_name: column_name)
 
-        fk_columns.each do |column_name|
-          if foreign_keys.none? { |row| row.fetch("column_name") == column_name }
-            agg.push(
-              {
-                table: table,
-                column_name: column_name,
-              }
-            )
-          end
+          # Skip columns that already have a foreign key constraint on this table.
+          next if fk_columns_for_table.include?(column_name)
+
+          # Skip columns that don't look like an FK candidate based on naming conventions.
+          next unless DetectFkColumn.call(column_name, all_tables)
+
+          # Rails polymorphic associations use <name>_id + <name>_type and can't have FK constraints.
+          candidate_prefix = column_name.delete_suffix("_id")
+          polymorphic_type_column = "#{candidate_prefix}_type"
+          # Skip polymorphic associations (cannot be expressed as a real FK constraint).
+          next if schema_column_names.include?(polymorphic_type_column)
+
+          column_name
         end
 
-        agg
+        candidate_fk_columns.each do |column_name|
+          agg.push(
+            {
+              table: table,
+              column_name: column_name,
+            }
+          )
+        end
       end
     end
 

lib/ruby_pg_extras/missing_fk_indexes.rb

@@ -7,26 +7,22 @@ def self.call(table_name)
     end
 
     def call(table_name)
+      indexes_info = query_module.indexes(in_format: :hash)
+      foreign_keys = query_module.foreign_keys(in_format: :hash)
+
       tables = if table_name
           [table_name]
         else
-          all_tables
+          foreign_keys.map { |row| row.fetch("table_name") }.uniq
         end
 
-      indexes_info = query_module.indexes(in_format: :hash)
-      schemas = query_module.table_schemas(in_format: :hash)
-
       tables.reduce([]) do |agg, table|
         index_info = indexes_info.select { |row| row.fetch("tablename") == table }
-        schema = schemas.select { |row| row.fetch("table_name") == table }
+        table_fks = foreign_keys.select { |row| row.fetch("table_name") == table }
 
-        fk_columns = schema.filter_map do |row|
-          if DetectFkColumn.call(row.fetch("column_name"), all_tables)
-            row.fetch("column_name")
-          end
-        end
+        table_fks.each do |fk|
+          column_name = fk.fetch("column_name")
 
-        fk_columns.each do |column_name|
           if index_info.none? { |row| row.fetch("columns").split(",").first == column_name }
             agg.push(
               {
@@ -43,10 +39,6 @@ def call(table_name)
 
     private
 
-    def all_tables
-      @_all_tables ||= query_module.table_size(in_format: :hash).map { |row| row.fetch("name") }
-    end
-
     def query_module
       RubyPgExtras
     end

spec/detect_fk_column_spec.rb

@@ -34,5 +34,41 @@
         expect(result).to eq(true)
       end
     end
+
+    context "matching table with underscored prefix" do
+      let(:column_name) { "account_user_id" }
+      let(:tables) { ["account_users", "users"] }
+
+      it "returns true" do
+        expect(result).to eq(true)
+      end
+    end
+
+    context "matching table with uncountable noun" do
+      let(:column_name) { "fish_id" }
+      let(:tables) { ["fish", "users"] }
+
+      it "returns true" do
+        expect(result).to eq(true)
+      end
+    end
+
+    context "matching table with mixed-case column name" do
+      let(:column_name) { "User_id" }
+      let(:tables) { ["users", "posts"] }
+
+      it "returns true" do
+        expect(result).to eq(true)
+      end
+    end
+
+    context "matching table with irregular plural already pluralized" do
+      let(:column_name) { "people_id" }
+      let(:tables) { ["people", "users"] }
+
+      it "returns true" do
+        expect(result).to eq(true)
+      end
+    end
   end
 end

spec/ignore_list_spec.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+require "spec_helper"
+require "ruby-pg-extras"
+
+describe RubyPgExtras::IgnoreList do
+  describe "#ignored?" do
+    it "returns false when ignore_list is nil" do
+      list = described_class.new(nil)
+      expect(list.ignored?(table: "posts", column_name: "topic_id")).to eq(false)
+    end
+
+    it "supports '*' to ignore everything" do
+      list = described_class.new(["*"])
+      expect(list.ignored?(table: "posts", column_name: "topic_id")).to eq(true)
+      expect(list.ignored?(table: "users", column_name: "customer_id")).to eq(true)
+    end
+
+    it "supports 'table.*' to ignore all columns for a table" do
+      list = described_class.new(["posts.*"])
+      expect(list.ignored?(table: "posts", column_name: "topic_id")).to eq(true)
+      expect(list.ignored?(table: "posts", column_name: "user_id")).to eq(true)
+      expect(list.ignored?(table: "users", column_name: "topic_id")).to eq(false)
+    end
+
+    it "supports 'column' to ignore a column name globally" do
+      list = described_class.new(["topic_id"])
+      expect(list.ignored?(table: "posts", column_name: "topic_id")).to eq(true)
+      expect(list.ignored?(table: "users", column_name: "topic_id")).to eq(true)
+      expect(list.ignored?(table: "posts", column_name: "user_id")).to eq(false)
+    end
+
+    it "supports 'table.column' to ignore a specific table+column" do
+      list = described_class.new(["posts.topic_id"])
+      expect(list.ignored?(table: "posts", column_name: "topic_id")).to eq(true)
+      expect(list.ignored?(table: "users", column_name: "topic_id")).to eq(false)
+      expect(list.ignored?(table: "posts", column_name: "user_id")).to eq(false)
+    end
+
+    it "accepts ignore_list as a comma-separated string" do
+      list = described_class.new("posts.topic_id, customer_id")
+      expect(list.ignored?(table: "posts", column_name: "topic_id")).to eq(true)
+      expect(list.ignored?(table: "users", column_name: "customer_id")).to eq(true)
+      expect(list.ignored?(table: "users", column_name: "topic_id")).to eq(false)
+    end
+
+    it "strips whitespace, drops empty entries and de-duplicates" do
+      list = described_class.new([" posts.topic_id ", "", "posts.topic_id", "  "])
+      expect(list.ignored?(table: "posts", column_name: "topic_id")).to eq(true)
+      expect(list.ignored?(table: "users", column_name: "topic_id")).to eq(false)
+    end
+
+    it "raises ArgumentError for unsupported ignore_list types" do
+      expect { described_class.new(123) }.to raise_error(ArgumentError)
+      expect { described_class.new({}) }.to raise_error(ArgumentError)
+    end
+  end
+end
+
+