From 1e802a3bd96b346beabca20cbcaa58e230238377 Mon Sep 17 00:00:00 2001 From: Harry Lascelles Date: Thu, 11 Jun 2026 11:51:37 +0100 Subject: [PATCH] Add mismatched sequence size check In Postgres, sequences can be of different sizes, for example, `integer` and `bigint`. There is essentially no reason ever to have a sequence that will have a smaller size than the primary key that it is vending IDs for. This PR adds a check to make sure that the sequence type matches that of the primary key ID column. Without it, a user may have a database column, which is `bigint`, and a sequence which is `integer` and will overflow unexpectedly. --- lib/active_record_doctor.rb | 1 + lib/active_record_doctor/config/default.rb | 4 + .../mismatched_sequence_type_test.rb | 147 ++++++++++++++++++ 3 files changed, 152 insertions(+) create mode 100644 test/active_record_doctor/detectors/mismatched_sequence_type_test.rb diff --git a/lib/active_record_doctor.rb b/lib/active_record_doctor.rb index 8a7b4b26..3dd311c8 100644 --- a/lib/active_record_doctor.rb +++ b/lib/active_record_doctor.rb @@ -20,6 +20,7 @@ require "active_record_doctor/detectors/incorrect_dependent_option" require "active_record_doctor/detectors/short_primary_key_type" require "active_record_doctor/detectors/mismatched_foreign_key_type" +require "active_record_doctor/detectors/mismatched_sequence_type" require "active_record_doctor/detectors/table_without_primary_key" require "active_record_doctor/detectors/table_without_timestamps" require "active_record_doctor/errors" diff --git a/lib/active_record_doctor/config/default.rb b/lib/active_record_doctor/config/default.rb index 4019d79c..45aafbce 100644 --- a/lib/active_record_doctor/config/default.rb +++ b/lib/active_record_doctor/config/default.rb @@ -34,6 +34,10 @@ ignore_tables: [], ignore_columns: [] + detector :mismatched_sequence_type, + enabled: true, + ignore_tables: [] + detector :missing_foreign_keys, enabled: true, ignore_models: [], diff --git a/test/active_record_doctor/detectors/mismatched_sequence_type_test.rb b/test/active_record_doctor/detectors/mismatched_sequence_type_test.rb new file mode 100644 index 00000000..e7e55f3c --- /dev/null +++ b/test/active_record_doctor/detectors/mismatched_sequence_type_test.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +class ActiveRecordDoctor::Detectors::MismatchedSequenceTypeTest < Minitest::Test + def test_non_postgresql_generates_no_errors + skip if postgresql? + + Context.create_table(:companies, id: :integer) + refute_problems + end + + def test_matching_integer_primary_key_and_sequence_is_not_reported + skip unless postgresql? + + Context.create_table(:companies, id: :integer) + refute_problems + end + + def test_matching_bigint_primary_key_and_sequence_is_not_reported + skip unless postgresql? + + Context.create_table(:companies, id: :bigint) + refute_problems + end + + def test_bigint_column_with_integer_sequence_is_reported + skip unless postgresql? + + Context.create_table(:companies, id: :integer) + # Widen the column without changing the sequence so the types diverge. + @connection.execute("ALTER TABLE companies ALTER COLUMN id TYPE bigint") + + sequence = fetch_sequence("companies", "id") + assert sequence, "Expected a sequence to exist for companies.id" + + assert_problems(<<~OUTPUT) + the sequence #{sequence} has type integer but companies.id is bigint - change the sequence type to bigint + OUTPUT + end + + def test_bigint_column_with_smallint_sequence_is_reported + skip unless postgresql? + + # Create with integer (gets a sequence), downgrade sequence to smallint, + # then widen the column to bigint — leaving a too-small sequence behind. + Context.create_table(:companies, id: :integer) + sequence = fetch_sequence("companies", "id") + assert sequence, "Expected a sequence to exist for companies.id" + @connection.execute("ALTER SEQUENCE #{sequence} AS smallint MAXVALUE 32767 RESTART 1") + @connection.execute("ALTER TABLE companies ALTER COLUMN id TYPE bigint") + + assert_problems(<<~OUTPUT) + the sequence #{sequence} has type smallint but companies.id is bigint - change the sequence type to bigint + OUTPUT + end + + def test_integer_column_with_smallint_sequence_is_reported + skip unless postgresql? + + # Create with integer (gets a sequence), then downgrade the sequence type + # to smallint — the column remains integer but the sequence is now too small. + Context.create_table(:companies, id: :integer) + sequence = fetch_sequence("companies", "id") + assert sequence, "Expected a sequence to exist for companies.id" + @connection.execute("ALTER SEQUENCE #{sequence} AS smallint MAXVALUE 32767 RESTART 1") + + assert_problems(<<~OUTPUT) + the sequence #{sequence} has type smallint but companies.id is integer - change the sequence type to integer + OUTPUT + end + + def test_uuid_primary_key_is_not_reported + skip unless postgresql? + + require_uuid_column_type! + + Context.create_table(:companies, id: :uuid) + refute_problems + end + + def test_no_primary_key_is_not_reported + skip unless postgresql? + + Context.create_table(:companies, id: false) do |t| + t.string :name, null: false + end + + refute_problems + end + + def test_config_ignore_tables + skip unless postgresql? + + Context.create_table(:companies, id: :integer) + @connection.execute("ALTER TABLE companies ALTER COLUMN id TYPE bigint") + + config_file(<<-CONFIG) + ActiveRecordDoctor.configure do |config| + config.detector :mismatched_sequence_type, + ignore_tables: ["companies"] + end + CONFIG + + refute_problems + end + + def test_global_ignore_tables + skip unless postgresql? + + Context.create_table(:companies, id: :integer) + @connection.execute("ALTER TABLE companies ALTER COLUMN id TYPE bigint") + + config_file(<<-CONFIG) + ActiveRecordDoctor.configure do |config| + config.global :ignore_tables, ["companies"] + end + CONFIG + + refute_problems + end + + private + + def setup + @connection = ActiveRecord::Base.connection + super + end + + def fetch_sequence(table, column) + @connection.select_value(<<~SQL) + SELECT n.nspname || '.' || seq.relname + FROM pg_class seq + JOIN pg_namespace n ON n.oid = seq.relnamespace + JOIN pg_depend dep + ON dep.objid = seq.oid + AND dep.classid = 'pg_class'::regclass + AND dep.refobjid = #{@connection.quote(table)}::regclass + AND dep.refobjsubid = ( + SELECT attnum FROM pg_attribute + WHERE attrelid = #{@connection.quote(table)}::regclass + AND attname = #{@connection.quote(column)} + ) + AND dep.deptype IN ('a', 'i') + WHERE seq.relkind = 'S' + LIMIT 1 + SQL + end +end