Skip to content

Commit

Permalink
Fix copying indexes with long names when changing column type
Browse files Browse the repository at this point in the history
Fixes #44.
  • Loading branch information
fatkodima committed Nov 15, 2023
1 parent b48b984 commit 2e427dc
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## master (unreleased)

- Fix copying indexes with long names when changing column type
- Enhance error messages with the link to the detailed description

## 0.9.2 (2023-11-02)
Expand Down
4 changes: 4 additions & 0 deletions lib/online_migrations/change_column_type_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,10 @@ def __copy_indexes(table_name, from_column, to_column)

name = index.name.gsub(from_column, to_column)

# Generate a shorter name if needed.
max_identifier_length = 63 # could use just `max_identifier_length` method for ActiveRecord >= 5.0.
name = index_name(table_name, new_columns) if name.length > max_identifier_length

options = {
unique: index.unique,
name: name,
Expand Down
19 changes: 1 addition & 18 deletions lib/online_migrations/command_checker.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require "erb"
require "openssl"
require "set"

module OnlineMigrations
Expand Down Expand Up @@ -616,30 +615,14 @@ def add_check_constraint(table_name, expression, **options)
def add_unique_constraint(table_name, column_name = nil, **options)
return if new_or_small_table?(table_name) || options[:using_index] || !column_name

index_name = index_name(table_name, column_name)
index_name = Utils.index_name(table_name, column_name)

raise_error :add_unique_constraint,
add_index_code: command_str(:add_index, table_name, column_name, unique: true, name: index_name, algorithm: :concurrently),
add_code: command_str(:add_unique_constraint, table_name, **options.merge(using_index: index_name)),
remove_code: command_str(:remove_unique_constraint, table_name, column_name)
end

# Implementation is from Active Record
def index_name(table_name, column_name)
max_index_name_size = 62
name = "index_#{table_name}_on_#{Array(column_name) * '_and_'}"
return name if name.bytesize <= max_index_name_size

# Fallback to short version, add hash to ensure uniqueness
hashed_identifier = "_#{OpenSSL::Digest::SHA256.hexdigest(name).first(10)}"
name = "idx_on_#{Array(column_name) * '_'}"

short_limit = max_index_name_size - hashed_identifier.bytesize
short_name = name[0, short_limit]

"#{short_name}#{hashed_identifier}"
end

def validate_constraint(*)
if crud_blocked?
raise_error :validate_constraint
Expand Down
16 changes: 16 additions & 0 deletions lib/online_migrations/schema_statements.rb
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,22 @@ def remove_index(table_name, column_name = nil, **options)
end
end

# @private
# From ActiveRecord. Will not be needed for ActiveRecord >= 7.1.
def index_name(table_name, options)
if options.is_a?(Hash)
if options[:column]
Utils.index_name(table_name, options[:column])
elsif options[:name]
options[:name]
else
raise ArgumentError, "You must specify the index name"
end
else
index_name(table_name, column: options)
end
end

# Extends default method to be idempotent and accept `:validate` option for Active Record < 5.2.
#
# @see https://edgeapi.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-add_foreign_key
Expand Down
19 changes: 19 additions & 0 deletions lib/online_migrations/utils.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require "openssl"

module OnlineMigrations
# @private
module Utils
Expand Down Expand Up @@ -78,6 +80,23 @@ def foreign_table_name(ref_name, options)
end
end

# Implementation is from ActiveRecord.
# This is not needed for ActiveRecord < 7.1 (https://github.com/rails/rails/pull/47753).
def index_name(table_name, column_name)
max_index_name_size = 62
name = "index_#{table_name}_on_#{Array(column_name) * '_and_'}"
return name if name.bytesize <= max_index_name_size

# Fallback to short version, add hash to ensure uniqueness
hashed_identifier = "_#{OpenSSL::Digest::SHA256.hexdigest(name).first(10)}"
name = "idx_on_#{Array(column_name) * '_'}"

short_limit = max_index_name_size - hashed_identifier.bytesize
short_name = name[0, short_limit]

"#{short_name}#{hashed_identifier}"
end

def ar_partial_writes?
ActiveRecord::Base.public_send(ar_partial_writes_setting)
end
Expand Down
15 changes: 15 additions & 0 deletions test/schema_statements/changing_column_type_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,21 @@ def test_finalize_column_type_change_copies_indexes
assert_equal(indexes_count, @connection.indexes(:projects).count { |index| index.columns.include?("name_for_type_change") })
end

def test_finalize_column_type_change_copies_indexes_with_long_names
max_identifier_length = 63 # could use just `max_identifier_length` method for ActiveRecord >= 5.0.
long_column_name = "a" * (max_identifier_length - "index_projects_on_".length)

@connection.change_table(:projects) do |t|
t.text long_column_name
t.index long_column_name, name: "index_projects_on_#{long_column_name}"
end

@connection.initialize_column_type_change(:projects, long_column_name, :string)
@connection.finalize_column_type_change(:projects, long_column_name)

assert_equal(1, @connection.indexes(:projects).count { |index| index.columns.include?("#{long_column_name}_for_type_change") })
end

def test_finalize_column_type_change_copies_foreign_key
@connection.initialize_column_type_change(:projects, :user_id, :bigint)
@connection.finalize_column_type_change(:projects, :user_id)
Expand Down

0 comments on commit 2e427dc

Please sign in to comment.