Generate index names less than 63 bytes (#94)

Addresses #89

lib/pg_ha_migrations/relation.rb

@@ -96,9 +96,23 @@ def partitions(include_sub_partitions: false, include_self: false)
   end
 
   class Index < Relation
-    # TODO: implement shortening to ensure < 63 bytes
+    MAX_NAME_SIZE = 63 # bytes
+
     def self.from_table_and_columns(table, columns)
-      new(connection.index_name(table.name, columns), table)
+      name = connection.index_name(table.name, columns)
+
+      # modified from https://github.com/rails/rails/pull/47753
+      if name.bytesize > MAX_NAME_SIZE
+        hashed_identifier = "_#{OpenSSL::Digest::SHA256.hexdigest(name).first(10)}"
+        description = name.sub("index_#{table.name}_on", "idx_on")
+
+        short_limit = MAX_NAME_SIZE - hashed_identifier.bytesize
+        short_description = description.mb_chars.limit(short_limit).to_s
+
+        name = "#{short_description}#{hashed_identifier}"
+      end
+
+      new(name, table)
     end
 
     attr_accessor :table

lib/pg_ha_migrations/unsafe_statements.rb

@@ -80,6 +80,16 @@ def unsafe_add_index(table, column_names, options = {})
       raise PgHaMigrations::InvalidMigrationError, "ActiveRecord drops the :opclass option when supplying a string containing an expression or list of columns; instead either supply an array of columns or include the opclass in the string for each column"
     end
 
+    validated_table = PgHaMigrations::Table.from_table_name(table)
+
+    validated_index = if options[:name]
+      PgHaMigrations::Index.new(options[:name], validated_table)
+    else
+      PgHaMigrations::Index.from_table_and_columns(validated_table, column_names)
+    end
+
+    options[:name] = validated_index.name
+
     execute_ancestor_statement(:add_index, table, column_names, **options)
   end
 

spec/safe_statements_spec.rb

@@ -1278,7 +1278,39 @@ def up
 
             indexes = ActiveRecord::Base.connection.indexes("foos")
             expect(indexes.size).to eq(1)
-            expect(indexes.first).to have_attributes(:table => "foos", :name => "index_foos_on_bar", :columns => ["bar"])
+            expect(indexes.first).to have_attributes(
+              table: "foos",
+              name: "index_foos_on_bar",
+              columns: ["bar"],
+            )
+          end
+
+          it "generates index name with hashed identifier when default index name is too large" do
+            setup_migration = Class.new(migration_klass) do
+              def up
+                unsafe_create_table "x" * 51
+                unsafe_add_column "x" * 51, :bar, :text
+              end
+            end
+            setup_migration.suppress_messages { setup_migration.migrate(:up) }
+
+            test_migration = Class.new(migration_klass) do
+              def up
+                safe_add_concurrent_index "x" * 51, [:bar]
+              end
+            end
+
+            expect do
+              test_migration.suppress_messages { test_migration.migrate(:up) }
+            end.to make_database_queries(matching: /CREATE +INDEX CONCURRENTLY/, count: 1)
+
+            indexes = ActiveRecord::Base.connection.indexes("x" * 51)
+            expect(indexes.size).to eq(1)
+            expect(indexes.first).to have_attributes(
+              table: "x" * 51,
+              name: "idx_on_bar_d7a594ad66",
+              columns: ["bar"],
+            )
           end
         end
 
@@ -1778,41 +1810,40 @@ def up
             end.to raise_error(PgHaMigrations::InvalidMigrationError, "Unexpected state. Parent index \"index_foos3_on_updated_at\" is invalid")
           end
 
-          it "raises error when parent index name is too large" do
-            create_range_partitioned_table(:foos3, migration_klass)
+          it "generates index name with hashed identifier when default child index name is too large" do
+            create_range_partitioned_table("x" * 42, migration_klass, with_partman: true)
 
             test_migration = Class.new(migration_klass) do
               def up
-                safe_add_concurrent_partitioned_index :foos3, :updated_at, name: "x" * 64
+                safe_add_concurrent_partitioned_index "x" * 42, :updated_at
               end
             end
 
-            expect do
-              test_migration.suppress_messages { test_migration.migrate(:up) }
-            end.to raise_error(
-              ArgumentError,
-              "Index name '#{"x" * 64}' on table 'foos3' is too long; the limit is 63 characters"
-            )
-          end
+            allow(ActiveRecord::Base.connection).to receive(:execute).and_call_original
 
-          it "raises error when child index name is too large" do
-            if ActiveRecord::VERSION::MAJOR >= 7 && ActiveRecord::VERSION::MINOR >= 1
-              skip "Rails 7.1+ will automatically generate index names less than 63 bytes"
+            aggregate_failures do
+              expect(ActiveRecord::Base.connection).to receive(:execute).with(/CREATE INDEX "index_#{"x" * 42}_on_updated_at" ON ONLY/).once.ordered
+              expect(ActiveRecord::Base.connection).to receive(:execute).with(/CREATE INDEX CONCURRENTLY "idx_on_updated_at_\w{10}/).exactly(10).times.ordered
+              expect(ActiveRecord::Base.connection).to receive(:execute).with(/ALTER INDEX .+\nATTACH PARTITION/).exactly(10).times.ordered
             end
 
-            create_range_partitioned_table("x" * 43, migration_klass, with_partman: true)
+            test_migration.suppress_messages { test_migration.migrate(:up) }
+          end
+
+          it "raises error when parent index name is too large" do
+            create_range_partitioned_table(:foos3, migration_klass)
 
             test_migration = Class.new(migration_klass) do
               def up
-                safe_add_concurrent_partitioned_index "x" * 43, :updated_at
+                safe_add_concurrent_partitioned_index :foos3, :updated_at, name: "x" * 64
               end
             end
 
             expect do
               test_migration.suppress_messages { test_migration.migrate(:up) }
             end.to raise_error(
               ArgumentError,
-              /Index name 'index_#{"x" * 43}_.+_on_updated_at' on table '#{"x" * 43}_.+' is too long; the limit is 63 characters/
+              "Index name '#{"x" * 64}' on table 'foos3' is too long; the limit is 63 characters"
             )
           end
 
@@ -1868,6 +1899,46 @@ def up
               migration.suppress_messages { migration.migrate(:up) }
             end.not_to make_database_queries(matching: /int4_ops/)
           end
+
+          it "generates index name with hashed identifier when default index name is too large" do
+            setup_migration = Class.new(migration_klass) do
+              def up
+                unsafe_create_table "x" * 51
+                unsafe_add_column "x" * 51, :bar, :text
+              end
+            end
+            setup_migration.suppress_messages { setup_migration.migrate(:up) }
+
+            test_migration = Class.new(migration_klass) do
+              def up
+                unsafe_add_index "x" * 51, [:bar]
+              end
+            end
+
+            expect do
+              test_migration.suppress_messages { test_migration.migrate(:up) }
+            end.to make_database_queries(matching: /CREATE INDEX "idx_on_bar_d7a594ad66"/, count: 1)
+
+            indexes = ActiveRecord::Base.connection.indexes("x" * 51)
+            expect(indexes.size).to eq(1)
+            expect(indexes.first).to have_attributes(
+              table: "x" * 51,
+              name: "idx_on_bar_d7a594ad66",
+              columns: ["bar"],
+            )
+          end
+
+          it "raises error when table does not exist" do
+            test_migration = Class.new(migration_klass) do
+              def up
+                unsafe_add_index :foo, :bar
+              end
+            end
+
+            expect do
+              test_migration.suppress_messages { test_migration.migrate(:up) }
+            end.to raise_error(PgHaMigrations::UndefinedTableError, "Table \"foo\" does not exist in search path")
+          end
         end
 
         describe "safe_remove_concurrent_index" do