Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider options when removing UNIQUE KEYs #47647

Merged
merged 1 commit into from Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -50,7 +50,7 @@ def visit_ExclusionConstraintDefinition(o)
end

def visit_UniqueKeyDefinition(o)
column_name = Array(o.columns).map { |column| quote_column_name(column) }.join(", ")
column_name = Array(o.column).map { |column| quote_column_name(column) }.join(", ")

sql = ["CONSTRAINT"]
sql << quote_column_name(o.name)
Expand Down
Expand Up @@ -211,7 +211,7 @@ def export_name_on_schema_dump?
end
end

UniqueKeyDefinition = Struct.new(:table_name, :columns, :options) do
UniqueKeyDefinition = Struct.new(:table_name, :column, :options) do
def name
options[:name]
end
Expand All @@ -227,6 +227,12 @@ def using_index
def export_name_on_schema_dump?
!ActiveRecord::SchemaDumper.unique_ignore_pattern.match?(name) if name
end

def defined_for?(name: nil, column: nil, **options)
(name.nil? || self.name == name.to_s) &&
(column.nil? || Array(self.column) == Array(column).map(&:to_s)) &&
options.all? { |k, v| self.options[k].to_s == v.to_s }
end
end

# = Active Record PostgreSQL Adapter \Table Definition
Expand Down
Expand Up @@ -54,7 +54,7 @@ def unique_keys_in_create(table, stream)
if (unique_keys = @connection.unique_keys(table)).any?
add_unique_key_statements = unique_keys.map do |unique_key|
parts = [
"t.unique_key #{unique_key.columns.inspect}"
"t.unique_key #{unique_key.column.inspect}"
]

parts << "deferrable: #{unique_key.deferrable.inspect}" if unique_key.deferrable
Expand Down
Expand Up @@ -747,7 +747,7 @@ def unique_key_options(table_name, column_name, options) # :nodoc:
end

options = options.dup
options[:name] ||= unique_key_name(table_name, column_name: column_name, **options)
options[:name] ||= unique_key_name(table_name, column: column_name, **options)
options
end

Expand All @@ -759,7 +759,7 @@ def unique_key_options(table_name, column_name, options) # :nodoc:
# to provide this in a migration's +change+ method so it can be reverted.
# In that case, +column_name+ will be used by #add_unique_key.
def remove_unique_key(table_name, column_name = nil, **options)
unique_name_to_delete = unique_key_for!(table_name, column_name: column_name, **options).name
unique_name_to_delete = unique_key_for!(table_name, column: column_name, **options).name

at = create_alter_table(table_name)
at.drop_unique_key(unique_name_to_delete)
Expand Down Expand Up @@ -1030,22 +1030,22 @@ def exclusion_constraint_for!(table_name, expression: nil, **options)

def unique_key_name(table_name, **options)
options.fetch(:name) do
column_name_or_index_name = options.fetch(:column_name) || options[:using_index]
identifier = "#{table_name}_#{column_name_or_index_name}_unique"
column_or_index = Array(options[:column] || options[:using_index]).map(&:to_s)
identifier = "#{table_name}_#{column_or_index * '_and_'}_unique"
hashed_identifier = Digest::SHA256.hexdigest(identifier).first(10)

"uniq_rails_#{hashed_identifier}"
end
end

def unique_key_for(table_name, **options)
unique_key_name = unique_key_name(table_name, **options)
unique_keys(table_name).detect { |unique_key| unique_key.name == unique_key_name }
name = unique_key_name(table_name, **options) unless options.key?(:column)
unique_keys(table_name).detect { |unique_key| unique_key.defined_for?(name: name, **options) }
end

def unique_key_for!(table_name, column_name: nil, **options)
unique_key_for(table_name, column_name: column_name, **options) ||
raise(ArgumentError, "Table '#{table_name}' has no unique constraint for #{column_name || options}")
def unique_key_for!(table_name, column: nil, **options)
unique_key_for(table_name, column: column, **options) ||
raise(ArgumentError, "Table '#{table_name}' has no unique constraint for #{column || options}")
end

def data_source_sql(name = nil, type: nil)
Expand Down
35 changes: 20 additions & 15 deletions activerecord/test/cases/migration/unique_key_test.rb
Expand Up @@ -30,15 +30,15 @@ def test_unique_keys
{
name: "test_unique_keys_position_deferrable_false",
deferrable: false,
columns: ["position_1"]
column: ["position_1"]
}, {
name: "test_unique_keys_position_deferrable_immediate",
deferrable: :immediate,
columns: ["position_2"]
column: ["position_2"]
}, {
name: "test_unique_keys_position_deferrable_deferred",
deferrable: :deferred,
columns: ["position_3"]
column: ["position_3"]
}
]

Expand All @@ -48,7 +48,7 @@ def test_unique_keys
constraint = unique_keys.find { |constraint| constraint.name == expected_constraint[:name] }
assert_equal "test_unique_keys", constraint.table_name
assert_equal expected_constraint[:name], constraint.name
assert_equal expected_constraint[:columns], constraint.columns
assert_equal expected_constraint[:column], constraint.column
assert_equal expected_constraint[:deferrable], constraint.deferrable
end
end
Expand All @@ -75,7 +75,7 @@ def test_add_unique_key_without_deferrable

constraint = unique_keys.first
assert_equal "sections", constraint.table_name
assert_equal "uniq_rails_3d89d7e853", constraint.name
assert_equal "uniq_rails_1e07660b77", constraint.name
assert_equal false, constraint.deferrable
end

Expand All @@ -87,7 +87,7 @@ def test_add_unique_key_with_deferrable_false

constraint = unique_keys.first
assert_equal "sections", constraint.table_name
assert_equal "uniq_rails_3d89d7e853", constraint.name
assert_equal "uniq_rails_1e07660b77", constraint.name
assert_equal false, constraint.deferrable
end

Expand All @@ -99,7 +99,7 @@ def test_add_unique_key_with_deferrable_immediate

constraint = unique_keys.first
assert_equal "sections", constraint.table_name
assert_equal "uniq_rails_3d89d7e853", constraint.name
assert_equal "uniq_rails_1e07660b77", constraint.name
assert_equal :immediate, constraint.deferrable
end

Expand All @@ -111,7 +111,7 @@ def test_add_unique_key_with_deferrable_deferred

constraint = unique_keys.first
assert_equal "sections", constraint.table_name
assert_equal "uniq_rails_3d89d7e853", constraint.name
assert_equal "uniq_rails_1e07660b77", constraint.name
assert_equal :deferred, constraint.deferrable
end

Expand Down Expand Up @@ -157,7 +157,7 @@ def test_add_unique_key_with_name_and_using_index
constraint = unique_keys.first
assert_equal "sections", constraint.table_name
assert_equal "unique_constraint", constraint.name
assert_equal ["position"], constraint.columns
assert_equal ["position"], constraint.column
assert_equal :immediate, constraint.deferrable
end

Expand All @@ -171,7 +171,7 @@ def test_add_unique_key_with_only_using_index
constraint = unique_keys.first
assert_equal "sections", constraint.table_name
assert_equal "uniq_rails_79b901ffb4", constraint.name
assert_equal ["position"], constraint.columns
assert_equal ["position"], constraint.column
assert_equal false, constraint.deferrable
end

Expand All @@ -184,16 +184,21 @@ def test_add_unique_key_with_columns_and_using_index
end

def test_remove_unique_key
assert_equal 0, @connection.unique_keys("sections").size
@connection.add_unique_key :sections, [:position], name: :unique_section_position
assert_equal 1, @connection.unique_keys("sections").size
@connection.remove_unique_key :sections, name: :unique_section_position
assert_empty @connection.unique_keys("sections")
end

@connection.add_unique_key :sections, [:position], name: "unique_section_position"
def test_remove_unique_key_by_column
@connection.add_unique_key :sections, [:position]
assert_equal 1, @connection.unique_keys("sections").size
@connection.remove_unique_key :sections, name: "unique_section_position"
assert_equal 0, @connection.unique_keys("sections").size
@connection.remove_unique_key :sections, [:position]
assert_empty @connection.unique_keys("sections")
end

def test_remove_non_existing_unique_key
assert_raises(ArgumentError) do
assert_raises(ArgumentError, match: /Table 'sections' has no unique constraint/) do
@connection.remove_unique_key :sections, name: "nonexistent"
end
end
Expand Down