From c590aeaf1b6a21fd813f7b956c7c3e7fa6548929 Mon Sep 17 00:00:00 2001 From: Erik Ogan Date: Mon, 17 Jun 2013 18:57:39 -0700 Subject: [PATCH 1/5] Add support for tables with outbound foreign key constraints. Brute force the new constraint names by offsetting the (optional) index in the name by the total number of keys. --- lib/lhm/connection.rb | 10 +---- lib/lhm/table.rb | 53 ++++++++++++++++++++++++++- spec/fixtures/fk_example.ddl | 6 +++ spec/integration/foreign_keys_spec.rb | 30 +++++++++++++++ 4 files changed, 90 insertions(+), 9 deletions(-) create mode 100644 spec/fixtures/fk_example.ddl create mode 100644 spec/integration/foreign_keys_spec.rb diff --git a/lib/lhm/connection.rb b/lib/lhm/connection.rb index b9b607df..ad3d8e49 100644 --- a/lib/lhm/connection.rb +++ b/lib/lhm/connection.rb @@ -59,10 +59,7 @@ def select_value(sql) end def destination_create(origin) - original = %{CREATE TABLE "#{ origin.name }"} - replacement = %{CREATE TABLE "#{ origin.destination_name }"} - - sql(origin.ddl.gsub(original, replacement)) + sql(origin.destination_ddl) end def execute(sql) @@ -125,10 +122,7 @@ def select_value(sql) end def destination_create(origin) - original = %{CREATE TABLE `#{ origin.name }`} - replacement = %{CREATE TABLE `#{ origin.destination_name }`} - - sql(origin.ddl.gsub(original, replacement)) + sql(origin.destination_ddl) end def execute(sql) diff --git a/lib/lhm/table.rb b/lib/lhm/table.rb index 1f7ed377..d18e7475 100644 --- a/lib/lhm/table.rb +++ b/lib/lhm/table.rb @@ -5,12 +5,13 @@ module Lhm class Table - attr_reader :name, :columns, :indices, :pk, :ddl + attr_reader :name, :columns, :indices, :constraints, :pk, :ddl def initialize(name, pk = "id", ddl = nil) @name = name @columns = {} @indices = {} + @constraints = {} @pk = pk @ddl = ddl end @@ -27,6 +28,27 @@ def self.parse(table_name, connection) Parser.new(table_name, connection).parse end + def destination_ddl + original = %r{CREATE TABLE ("|`)#{ name }\1} + # Strange substitutions are happening when I put this in the string directly + repl = '\1' + replacement = %Q{CREATE TABLE #{repl}#{ destination_name }#{repl}} + + dest = ddl + dest.gsub!(original, replacement) + + foreign_keys = constraints.select {|col, c| !c[:referenced_column].nil?} + + foreign_keys.keys.each_with_index do |key, i| + original = foreign_keys[key][:name] + # Offset the new key names by the total size so they cannot overlap + replacement = original.sub(/(_\d+)?$/, "_#{foreign_keys.size + i + 1}") + dest.gsub!(original, replacement) + end + + dest + end + class Parser include SqlHelper @@ -59,6 +81,10 @@ def parse extract_indices(read_indices).each do |idx, columns| table.indices[idx] = columns end + + extract_constraints(read_constraints).each do |data| + table.constraints[data[:column]] = data + end end end @@ -93,6 +119,31 @@ def extract_indices(indices) end end + def read_constraints + @connection.select_all %Q{ + select * + from information_schema.key_column_usage + where table_name = '#{ @table_name }' + and table_schema = '#{ @schema_name }' + } + end + + def extract_constraints(constraints) + constraints.map do |row| + constraint_name = struct_key(row, 'CONSTRAINT_NAME') + column_name = struct_key(row, "COLUMN_NAME") + ref_table_name = struct_key(row, 'REFERENCED_TABLE_NAME') + ref_col_name = struct_key(row, 'REFERENCED_COLUMN_NAME') + + { + :name => row[constraint_name], + :column => row[column_name], + :referenced_table => row[ref_table_name], + :referenced_column => row[ref_col_name] + } + end + end + def extract_primary_key(schema) cols = schema.select do |defn| column_key = struct_key(defn, "COLUMN_KEY") diff --git a/spec/fixtures/fk_example.ddl b/spec/fixtures/fk_example.ddl new file mode 100644 index 00000000..01880a27 --- /dev/null +++ b/spec/fixtures/fk_example.ddl @@ -0,0 +1,6 @@ +CREATE TABLE `fk_example` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `user_id` int(11) NOT NULL, + PRIMARY KEY (`id`), + CONSTRAINT `fk_example_ibfk_1` FOREIGN KEY (`user_id`) REFERENCES `users` (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8 diff --git a/spec/integration/foreign_keys_spec.rb b/spec/integration/foreign_keys_spec.rb new file mode 100644 index 00000000..1ffa8396 --- /dev/null +++ b/spec/integration/foreign_keys_spec.rb @@ -0,0 +1,30 @@ +# Copyright (c) 2011 - 2013, SoundCloud Ltd., Rany Keddo, Tobias Bielohlawek, Tobias +# Schmidt + +require File.expand_path(File.dirname(__FILE__)) + '/integration_helper' + +require 'lhm' + +describe Lhm do + include IntegrationHelper + + before(:each) { connect_master! } + + before(:each) do + # Be absolutely sure it is not there + execute 'drop table if exists fk_example' + table_create(:users) + table_create(:fk_example) + end + + after(:each) do + # Clean it up since it could cause trouble + execute 'drop table if exists fk_example' + end + + it 'should handle tables with foriegn keys' do + Lhm.change_table(:fk_example) do |t| + t.add_column(:new_column, "INT(12) DEFAULT '0'") + end + end +end From df01b8971ae1f152afddd7ad447d7dbdc5b654fb Mon Sep 17 00:00:00 2001 From: Erik Ogan Date: Tue, 18 Jun 2013 21:23:06 -0700 Subject: [PATCH 2/5] A much better way to ensure constraint name uniqueness Reads all of the constraint names and makes sure to choose one that does not clash. --- lib/lhm/table.rb | 93 ++++++++++++++++----- spec/fixtures/fk_example_non_sequential.ddl | 6 ++ spec/integration/foreign_keys_spec.rb | 41 +++++++-- spec/unit/table_spec.rb | 19 ++++- 4 files changed, 125 insertions(+), 34 deletions(-) create mode 100644 spec/fixtures/fk_example_non_sequential.ddl diff --git a/lib/lhm/table.rb b/lib/lhm/table.rb index d18e7475..ae0461f5 100644 --- a/lib/lhm/table.rb +++ b/lib/lhm/table.rb @@ -5,10 +5,11 @@ module Lhm class Table - attr_reader :name, :columns, :indices, :constraints, :pk, :ddl + attr_reader :schema, :name, :columns, :indices, :constraints, :pk, :ddl - def initialize(name, pk = "id", ddl = nil) + def initialize(name, schema = 'default', pk = "id", ddl = nil) @name = name + @schema = schema @columns = {} @indices = {} @constraints = {} @@ -41,14 +42,20 @@ def destination_ddl foreign_keys.keys.each_with_index do |key, i| original = foreign_keys[key][:name] - # Offset the new key names by the total size so they cannot overlap - replacement = original.sub(/(_\d+)?$/, "_#{foreign_keys.size + i + 1}") + replacement = replacement_constraint(original) dest.gsub!(original, replacement) end dest end + @@schema_constraints = {} + + def self.schema_constraints(schema, value = nil) + @@schema_constraints[schema] = value if value + @@schema_constraints[schema] + end + class Parser include SqlHelper @@ -65,7 +72,7 @@ def ddl def parse schema = read_information_schema - Table.new(@table_name, extract_primary_key(schema), ddl).tap do |table| + Table.new(@table_name, @schema_name, extract_primary_key(schema), ddl).tap do |table| schema.each do |defn| column_name = struct_key(defn, "COLUMN_NAME") column_type = struct_key(defn, "COLUMN_TYPE") @@ -82,9 +89,18 @@ def parse table.indices[idx] = columns end - extract_constraints(read_constraints).each do |data| - table.constraints[data[:column]] = data + constraints = {} + + extract_constraints(read_constraints(nil)).each do |data| + if data[:schema] == @schema_name && data[:table] == @table_name + table.constraints[data[:column]] = data + end + + next if data[:name] == 'PRIMARY' + constraints[data[:name]] = data end + + Table.schema_constraints(@schema_name, constraints) end end @@ -119,28 +135,44 @@ def extract_indices(indices) end end - def read_constraints - @connection.select_all %Q{ + def read_constraints(table = @table_name) + query = %Q{ select * from information_schema.key_column_usage - where table_name = '#{ @table_name }' - and table_schema = '#{ @schema_name }' + where table_schema = '#{ @schema_name }' } + query += %Q{ + and table_name = '#{ @table_name }' + } if table + + @connection.select_all(query) end def extract_constraints(constraints) + columns = %w{ + CONSTRAINT_NAME + TABLE_SCHEMA + TABLE_NAME + COLUMN_NAME + ORDINAL_POSITION + POSITION_IN_UNIQUE_CONSTRAINT + REFERENCED_TABLE_SCHEMA + REFERENCED_TABLE_NAME + REFERENCED_COLUMN_NAME + } + constraints.map do |row| - constraint_name = struct_key(row, 'CONSTRAINT_NAME') - column_name = struct_key(row, "COLUMN_NAME") - ref_table_name = struct_key(row, 'REFERENCED_TABLE_NAME') - ref_col_name = struct_key(row, 'REFERENCED_COLUMN_NAME') - - { - :name => row[constraint_name], - :column => row[column_name], - :referenced_table => row[ref_table_name], - :referenced_column => row[ref_col_name] - } + result = {} + columns.each do |c| + sym = c.dup + # The order of these substitutions is important + sym.gsub!(/CONSTRAINT_/, '') + sym.gsub!(/_NAME/, '') + sym.gsub!(/TABLE_/, '') + result[sym.downcase.to_sym] = row[struct_key(row, c)] + end + + result end end @@ -158,5 +190,22 @@ def extract_primary_key(schema) keys.length == 1 ? keys.first : keys end end + + private + + def replacement_constraint(name) + existing = Table.schema_constraints(@schema) + + seq = 1 + name = name.dup + + begin + name.sub!(/(_\d+)?$/, "_#{seq}") + seq += 1 + end while existing.has_key?(name) + + return name + end + end end diff --git a/spec/fixtures/fk_example_non_sequential.ddl b/spec/fixtures/fk_example_non_sequential.ddl new file mode 100644 index 00000000..b61db2a0 --- /dev/null +++ b/spec/fixtures/fk_example_non_sequential.ddl @@ -0,0 +1,6 @@ +CREATE TABLE `fk_example_non_sequential` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `user_id` int(11) NOT NULL, + PRIMARY KEY (`id`), + CONSTRAINT `fk_example_ibfk_2` FOREIGN KEY (`user_id`) REFERENCES `users` (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8 diff --git a/spec/integration/foreign_keys_spec.rb b/spec/integration/foreign_keys_spec.rb index 1ffa8396..a612cd77 100644 --- a/spec/integration/foreign_keys_spec.rb +++ b/spec/integration/foreign_keys_spec.rb @@ -11,20 +11,43 @@ before(:each) { connect_master! } before(:each) do - # Be absolutely sure it is not there - execute 'drop table if exists fk_example' + # Be absolutely sure none of these exist yet + Lhm.cleanup(true) + %w{fk_example fk_example_non_sequential}.each do |table| + execute "drop table if exists #{table}" + end + table_create(:users) - table_create(:fk_example) end - after(:each) do - # Clean it up since it could cause trouble - execute 'drop table if exists fk_example' + describe 'the simplest case' do + before(:each) do + table_create(:fk_example) + end + + after (:each) do + # Clean it up since it could cause trouble + execute 'drop table if exists fk_example' + Lhm.cleanup(true) + end + it 'should handle tables with foriegn keys' do + Lhm.change_table(:fk_example) do |t| + t.add_column(:new_column, "INT(12) DEFAULT '0'") + end + end end - it 'should handle tables with foriegn keys' do - Lhm.change_table(:fk_example) do |t| - t.add_column(:new_column, "INT(12) DEFAULT '0'") + describe 'the foreign key sequence number is not 1' do + before(:each) do + table_create(:fk_example_non_sequential) + end + + it 'should be able to create this table' do + Lhm.change_table(:fk_example_non_sequential) do |t| + t.add_column(:new_column, "INT(12) DEFAULT '0'") + end end end + + end diff --git a/spec/unit/table_spec.rb b/spec/unit/table_spec.rb index 285a4bc2..3974be61 100644 --- a/spec/unit/table_spec.rb +++ b/spec/unit/table_spec.rb @@ -15,19 +15,32 @@ end end + describe 'ddl' do + it "should build the destination table" do + table = "users" + schema = "default" + + @table = Lhm::Table.new(table, schema, "id", %Q{CREATE TABLE `#{table}` (random_constraint)}) + @table.constraints['user_id'] = {:name => 'random_constraint', :referenced_column => true} + Lhm::Table.schema_constraints(schema, {'random_constraint_1' => true}) + + @table.destination_ddl.must_equal %Q{CREATE TABLE `#{@table.destination_name}` (random_constraint_2)} + end + end + describe "constraints" do it "should be satisfied with a single column primary key called id" do - @table = Lhm::Table.new("table", "id") + @table = Lhm::Table.new("table", "default", "id") @table.satisfies_primary_key?.must_equal true end it "should not be satisfied with a primary key unless called id" do - @table = Lhm::Table.new("table", "uuid") + @table = Lhm::Table.new("table", "default", "uuid") @table.satisfies_primary_key?.must_equal false end it "should not be satisfied with multicolumn primary key" do - @table = Lhm::Table.new("table", ["id", "secondary"]) + @table = Lhm::Table.new("table", "default", ["id", "secondary"]) @table.satisfies_primary_key?.must_equal false end end From f8937a45a9394ba702fde9fd81ce61bbd2b1b70d Mon Sep 17 00:00:00 2001 From: Erik Ogan Date: Sat, 29 Jun 2013 17:58:45 -0700 Subject: [PATCH 3/5] Actually test that the foreign key constraint is still there, rather than just dropped. --- spec/integration/foreign_keys_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/spec/integration/foreign_keys_spec.rb b/spec/integration/foreign_keys_spec.rb index a612cd77..6475539a 100644 --- a/spec/integration/foreign_keys_spec.rb +++ b/spec/integration/foreign_keys_spec.rb @@ -34,6 +34,14 @@ Lhm.change_table(:fk_example) do |t| t.add_column(:new_column, "INT(12) DEFAULT '0'") end + + slave do + table_read(:fk_example).constraints['user_id'].slice(:name, :referenced_table, :referenced_column).must_equal({ + name: 'fk_example_ibfk_2', + referenced_table: 'users', + referenced_column: 'id' + }) + end end end @@ -46,6 +54,14 @@ Lhm.change_table(:fk_example_non_sequential) do |t| t.add_column(:new_column, "INT(12) DEFAULT '0'") end + + slave do + table_read(:fk_example_non_sequential).constraints['user_id'].slice(:name, :referenced_table, :referenced_column).must_equal({ + name: 'fk_example_ibfk_1', + referenced_table: 'users', + referenced_column: 'id' + }) + end end end From f760f1af7f51749fd1769edce26ca3fbd174c7f6 Mon Sep 17 00:00:00 2001 From: Erik Ogan Date: Sat, 29 Jun 2013 19:23:05 -0700 Subject: [PATCH 4/5] table constraints should only be foreign key constraints, since unique constraints are already stored in indices --- lib/lhm/table.rb | 2 +- spec/integration/table_spec.rb | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/lhm/table.rb b/lib/lhm/table.rb index ae0461f5..abc2bd38 100644 --- a/lib/lhm/table.rb +++ b/lib/lhm/table.rb @@ -96,7 +96,6 @@ def parse table.constraints[data[:column]] = data end - next if data[:name] == 'PRIMARY' constraints[data[:name]] = data end @@ -140,6 +139,7 @@ def read_constraints(table = @table_name) select * from information_schema.key_column_usage where table_schema = '#{ @schema_name }' + and referenced_column_name is not null } query += %Q{ and table_name = '#{ @table_name }' diff --git a/spec/integration/table_spec.rb b/spec/integration/table_spec.rb index 58905f88..f59518a4 100644 --- a/spec/integration/table_spec.rb +++ b/spec/integration/table_spec.rb @@ -43,6 +43,23 @@ indices["index_users_on_reference"]. must_equal(["reference"]) end + + it "should parse constraints" do + begin + @table = table_create(:fk_example) + @table.constraints.keys.must_equal %w{user_id} + + compare = { + name: "fk_example_ibfk_1", + referenced_table: "users", + referenced_column: "id" + } + + @table.constraints['user_id'].slice(*(compare.keys)).must_equal compare + ensure + execute 'drop table if exists fk_example' + end + end end end end From 8bdba82c2282eb73b6924701fd8f166a647970f0 Mon Sep 17 00:00:00 2001 From: Erik Ogan Date: Sat, 20 Jul 2013 10:22:05 -0700 Subject: [PATCH 5/5] Hash#slice is an ActiveSupport extension. Do not assume it is present. --- spec/integration/foreign_keys_spec.rb | 12 ++++++++---- spec/integration/integration_helper.rb | 10 ++++++++++ spec/integration/table_spec.rb | 4 ++-- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/spec/integration/foreign_keys_spec.rb b/spec/integration/foreign_keys_spec.rb index 6475539a..c5d43070 100644 --- a/spec/integration/foreign_keys_spec.rb +++ b/spec/integration/foreign_keys_spec.rb @@ -36,11 +36,13 @@ end slave do - table_read(:fk_example).constraints['user_id'].slice(:name, :referenced_table, :referenced_column).must_equal({ + actual = table_read(:fk_example).constraints['user_id'] + expected = { name: 'fk_example_ibfk_2', referenced_table: 'users', referenced_column: 'id' - }) + } + hash_slice(actual, expected.keys).must_equal(expected) end end end @@ -56,11 +58,13 @@ end slave do - table_read(:fk_example_non_sequential).constraints['user_id'].slice(:name, :referenced_table, :referenced_column).must_equal({ + actual = table_read(:fk_example_non_sequential).constraints['user_id'] + expected = { name: 'fk_example_ibfk_1', referenced_table: 'users', referenced_column: 'id' - }) + } + hash_slice(actual, expected.keys).must_equal(expected) end end end diff --git a/spec/integration/integration_helper.rb b/spec/integration/integration_helper.rb index d489e3fc..a88c6e8b 100644 --- a/spec/integration/integration_helper.rb +++ b/spec/integration/integration_helper.rb @@ -121,6 +121,16 @@ def table_exists?(table) connection.table_exists?(table.name) end + + def hash_slice(hash, keys) + if hash.respond_to?(:slice) + hash.slice(*keys) + else + check = {} + keys.each {|k| check[k] = hash[k]} + check + end + end # # Database Helpers # diff --git a/spec/integration/table_spec.rb b/spec/integration/table_spec.rb index f59518a4..ea1167ae 100644 --- a/spec/integration/table_spec.rb +++ b/spec/integration/table_spec.rb @@ -49,13 +49,13 @@ @table = table_create(:fk_example) @table.constraints.keys.must_equal %w{user_id} - compare = { + expected = { name: "fk_example_ibfk_1", referenced_table: "users", referenced_column: "id" } - @table.constraints['user_id'].slice(*(compare.keys)).must_equal compare + hash_slice(@table.constraints['user_id'], expected.keys).must_equal expected ensure execute 'drop table if exists fk_example' end