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

Dump indexes in create_table for generates SQL in one query #23557

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
7 changes: 7 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
* Dump indexes in `create_table` for generates SQL in one query.

If the adapter supports indexes in create table, it generates SQL
in one query.

*Ryuta Kamizono*

* SQLite: Fix uniqueness validation when values exceed the column limit.

SQLite doesn't impose length restrictions on strings, BLOBs, or numeric
Expand Down
53 changes: 33 additions & 20 deletions activerecord/lib/active_record/schema_dumper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,11 @@ def table(table, stream)
tbl.puts
end

indexes_in_create(table, tbl)

tbl.puts " end"
tbl.puts

indexes(table, tbl)

tbl.rewind
stream.print tbl.read
rescue => e
Expand All @@ -195,34 +195,47 @@ def table(table, stream)
stream
end

# Keep it for indexing materialized views
def indexes(table, stream)
if (indexes = @connection.indexes(table)).any?
add_index_statements = indexes.map do |index|
statement_parts = [
"add_index #{remove_prefix_and_suffix(index.table).inspect}",
index.columns.inspect,
"name: #{index.name.inspect}",
]
statement_parts << 'unique: true' if index.unique

index_lengths = (index.lengths || []).compact
statement_parts << "length: #{Hash[index.columns.zip(index.lengths)].inspect}" if index_lengths.any?

index_orders = index.orders || {}
statement_parts << "order: #{index.orders.inspect}" if index_orders.any?
statement_parts << "where: #{index.where.inspect}" if index.where
statement_parts << "using: #{index.using.inspect}" if index.using
statement_parts << "type: #{index.type.inspect}" if index.type
statement_parts << "comment: #{index.comment.inspect}" if index.comment

" #{statement_parts.join(', ')}"
table_name = remove_prefix_and_suffix(index.table).inspect
" add_index #{([table_name]+index_parts(index)).join(', ')}"
end

stream.puts add_index_statements.sort.join("\n")
stream.puts
end
end

def indexes_in_create(table, stream)
if (indexes = @connection.indexes(table)).any?
index_statements = indexes.map do |index|
" t.index #{index_parts(index).join(', ')}"
end
stream.puts index_statements.sort.join("\n")
end
end

def index_parts(index)
index_parts = [
index.columns.inspect,
"name: #{index.name.inspect}",
]
index_parts << 'unique: true' if index.unique

index_lengths = (index.lengths || []).compact
index_parts << "length: #{Hash[index.columns.zip(index.lengths)].inspect}" if index_lengths.any?

index_orders = index.orders || {}
index_parts << "order: #{index.orders.inspect}" if index_orders.any?
index_parts << "where: #{index.where.inspect}" if index.where
index_parts << "using: #{index.using.inspect}" if index.using
index_parts << "type: #{index.type.inspect}" if index.type
index_parts << "comment: #{index.comment.inspect}" if index.comment
index_parts
end

def foreign_keys(table, stream)
if (foreign_keys = @connection.foreign_keys(table)).any?
add_foreign_key_statements = foreign_keys.map do |foreign_key|
Expand Down
4 changes: 2 additions & 2 deletions activerecord/test/cases/comment_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ def test_schema_dump_with_comments
assert_match %r[t\.string\s+"obvious"\n], output
assert_match %r[t\.string\s+"content",\s+comment: "Whoa, content describes itself!"], output
assert_match %r[t\.integer\s+"rating",\s+comment: "I am running out of imagination"], output
assert_match %r[add_index\s+.+\s+comment: "\\\"Very important\\\" index that powers all the performance.\\nAnd it's fun!"], output
assert_match %r[add_index\s+.+\s+name: "idx_obvious",.+\s+comment: "We need to see obvious comments"], output
assert_match %r[t\.index\s+.+\s+comment: "\\\"Very important\\\" index that powers all the performance.\\nAnd it's fun!"], output
assert_match %r[t\.index\s+.+\s+name: "idx_obvious",.+\s+comment: "We need to see obvious comments"], output
end

def test_schema_dump_omits_blank_comments
Expand Down
20 changes: 10 additions & 10 deletions activerecord/test/cases/schema_dumper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,24 +171,24 @@ def test_schema_dump_with_regexp_ignored_table
end

def test_schema_dumps_index_columns_in_right_order
index_definition = standard_dump.split(/\n/).grep(/add_index.*companies/).first.strip
index_definition = standard_dump.split(/\n/).grep(/t\.index.*company_index/).first.strip
if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter)
assert_equal 'add_index "companies", ["firm_id", "type", "rating"], name: "company_index", using: :btree', index_definition
assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", using: :btree', index_definition
else
assert_equal 'add_index "companies", ["firm_id", "type", "rating"], name: "company_index"', index_definition
assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index"', index_definition
end
end

def test_schema_dumps_partial_indices
index_definition = standard_dump.split(/\n/).grep(/add_index.*company_partial_index/).first.strip
index_definition = standard_dump.split(/\n/).grep(/t\.index.*company_partial_index/).first.strip
if current_adapter?(:PostgreSQLAdapter)
assert_equal 'add_index "companies", ["firm_id", "type"], name: "company_partial_index", where: "(rating > 10)", using: :btree', index_definition
assert_equal 't.index ["firm_id", "type"], name: "company_partial_index", where: "(rating > 10)", using: :btree', index_definition
elsif current_adapter?(:Mysql2Adapter)
assert_equal 'add_index "companies", ["firm_id", "type"], name: "company_partial_index", using: :btree', index_definition
assert_equal 't.index ["firm_id", "type"], name: "company_partial_index", using: :btree', index_definition
elsif current_adapter?(:SQLite3Adapter) && ActiveRecord::Base.connection.supports_partial_index?
assert_equal 'add_index "companies", ["firm_id", "type"], name: "company_partial_index", where: "rating > 10"', index_definition
assert_equal 't.index ["firm_id", "type"], name: "company_partial_index", where: "rating > 10"', index_definition
else
assert_equal 'add_index "companies", ["firm_id", "type"], name: "company_partial_index"', index_definition
assert_equal 't.index ["firm_id", "type"], name: "company_partial_index"', index_definition
end
end

Expand Down Expand Up @@ -235,8 +235,8 @@ def test_schema_does_not_include_limit_for_emulated_mysql_boolean_fields

def test_schema_dumps_index_type
output = standard_dump
assert_match %r{add_index "key_tests", \["awesome"\], name: "index_key_tests_on_awesome", type: :fulltext}, output
assert_match %r{add_index "key_tests", \["pizza"\], name: "index_key_tests_on_pizza", using: :btree}, output
assert_match %r{t\.index \["awesome"\], name: "index_key_tests_on_awesome", type: :fulltext}, output
assert_match %r{t\.index \["pizza"\], name: "index_key_tests_on_pizza", using: :btree}, output
end
end

Expand Down