Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

fixes soundcloud#11

  • Loading branch information...
commit eecad6e650d55d9e3df18e160b26c48beaa6722c 1 parent 5f33f2c
@purzelrakete purzelrakete authored
View
6 CHANGELOG.md
@@ -1,3 +1,9 @@
+# 1.0.2 (Febuary 17, 2012)
+
+* closes https://github.com/soundcloud/large-hadron-migrator/issues/11
+ this critical bug could cause data loss. table parser was replaced with
+ an implementation that reads directly from information_schema.
+
# 1.0.1 (Febuary 09, 2012)
* released to rubygems
View
79 lib/lhm/table.rb
@@ -1,6 +1,8 @@
# Copyright (c) 2011, SoundCloud Ltd., Rany Keddo, Tobias Bielohlawek, Tobias
# Schmidt
+require 'lhm/sql_helper'
+
module Lhm
class Table
attr_reader :name, :columns, :indices, :pk, :ddl
@@ -22,58 +24,71 @@ def destination_name
end
def self.parse(table_name, connection)
- sql = "show create table `#{ table_name }`"
- ddl = connection.execute(sql).fetch_row.last
-
- Parser.new(ddl).parse
+ Parser.new(table_name, connection).parse
end
class Parser
- def initialize(ddl)
- @ddl = ddl
- end
+ include SqlHelper
- def lines
- @ddl.lines.to_a.map(&:strip).reject(&:empty?)
+ def initialize(table_name, connection)
+ @table_name = table_name.to_s
+ @schema_name = connection.current_database
+ @connection = connection
end
- def create_definitions
- lines[1..-2]
+ def ddl
+ sql = "show create table `#{ @table_name }`"
+ @connection.execute(sql).fetch_row.last
end
def parse
- _, name = *lines.first.match("`([^ ]*)`")
- pk_line = create_definitions.grep(primary).first
-
- if pk_line
- _, pk = *pk_line.match(primary)
- table = Table.new(name, pk, @ddl)
-
- create_definitions.each do |definition|
- case definition
- when index
- table.indices[$1] = { :metadata => $2 }
- when column
- table.columns[$1] = { :type => $2, :metadata => $3 }
- end
+ schema = read_information_schema
+
+ Table.new(@table_name, extract_primary_key(schema), ddl).tap do |table|
+ schema.each do |defn|
+ table.columns[defn["COLUMN_NAME"]] = {
+ :type => defn["COLUMN_TYPE"],
+ :is_nullable => defn["IS_NULLABLE"],
+ :column_default => defn["COLUMN_DEFAULT"]
+ }
end
- table
+ extract_indices(read_indices).each do |idx, columns|
+ table.indices[idx] = columns
+ end
end
end
private
- def primary
- /^PRIMARY KEY (?:USING (?:HASH|[BR]TREE) )?\(`([^ ]*)`\),?$/
+ def read_information_schema
+ @connection.select_all %Q{
+ select *
+ from information_schema.columns
+ where table_name = "#{ @table_name }"
+ and table_schema = "#{ @schema_name }"
+ }
end
- def index
- /^(?:UNIQUE )?(?:INDEX|KEY) `([^ ]*)` (.*?),?$/
+ def read_indices
+ @connection.select_all %Q{
+ show indexes from `#{ @schema_name }`.`#{ @table_name }`
+ where key_name != "PRIMARY"
+ }
+ end
+
+ def extract_indices(indices)
+ indices.map { |row| [row["Key_name"], row["Column_name"]] }.
+ inject(Hash.new { |h, k| h[k] = []}) do |memo, (idx, column)|
+ memo[idx] << column
+ memo
+ end
end
- def column
- /^`([^ ]*)` ([^ ]*) (.*?),?$/
+ def extract_primary_key(schema)
+ cols = schema.select { |defn| defn["COLUMN_KEY"] == "PRI" }
+ keys = cols.map { |defn| defn["COLUMN_NAME"] }
+ keys.length == 1 ? keys.first : keys
end
end
end
View
2  lib/lhm/version.rb
@@ -2,5 +2,5 @@
# Schmidt
module Lhm
- VERSION = "1.0.1"
+ VERSION = "1.0.2"
end
View
9 spec/integration/lhm_spec.rb
@@ -23,7 +23,8 @@
slave do
table_read(:users).columns["logins"].must_equal({
:type => "int(12)",
- :metadata => "DEFAULT '0'"
+ :is_nullable => "YES",
+ :column_default => '0'
})
end
end
@@ -98,7 +99,8 @@
slave do
table_read(:users).columns["flag"].must_equal({
:type => "tinyint(1)",
- :metadata => "DEFAULT NULL"
+ :is_nullable => "YES",
+ :column_default => nil
})
end
end
@@ -111,7 +113,8 @@
slave do
table_read(:users).columns["comment"].must_equal({
:type => "varchar(20)",
- :metadata => "NOT NULL DEFAULT 'none'"
+ :is_nullable => "NO",
+ :column_default => "none"
})
end
end
View
48 spec/integration/table_spec.rb
@@ -0,0 +1,48 @@
+# Copyright (c) 2011, SoundCloud Ltd., Rany Keddo, Tobias Bielohlawek, Tobias
+# Schmidt
+
+require File.expand_path(File.dirname(__FILE__)) + '/integration_helper'
+
+require 'lhm'
+require 'lhm/table'
+
+describe Lhm::Table do
+ include IntegrationHelper
+
+ describe Lhm::Table::Parser do
+ describe "create table parsing" do
+ before(:each) do
+ connect_master!
+ @table = Lhm::Table::Parser.new(:users, connection).parse
+ end
+
+ it "should parse table name in show create table" do
+ @table.name.must_equal("users")
+ end
+
+ it "should parse primary key" do
+ @table.pk.must_equal("id")
+ end
+
+ it "should parse column type in show create table" do
+ @table.columns["username"][:type].must_equal("varchar(255)")
+ end
+
+ it "should parse column metadata" do
+ @table.columns["username"][:column_default].must_equal nil
+ end
+
+ it "should parse indices" do
+ @table.
+ indices["index_users_on_username_and_created_at"].
+ must_equal(["username", "created_at"])
+ end
+
+ it "should parse index" do
+ @table.
+ indices["index_users_on_reference"].
+ must_equal(["reference"])
+ end
+ end
+ end
+end
View
45 spec/unit/table_spec.rb
@@ -9,11 +9,8 @@
include UnitHelper
describe "names" do
- before(:each) do
- @table = Lhm::Table::Parser.new(fixture("users.ddl")).parse
- end
-
it "should name destination" do
+ @table = Lhm::Table.new("users")
@table.destination_name.must_equal "lhmn_users"
end
end
@@ -34,44 +31,4 @@
@table.satisfies_primary_key?.must_equal false
end
end
-
- describe Lhm::Table::Parser do
- describe "create table parsing" do
- before(:each) do
- @table = Lhm::Table::Parser.new(fixture("users.ddl")).parse
- end
-
- it "should parse table name in show create table" do
- @table.name.must_equal("users")
- end
-
- it "should parse primary key" do
- @table.pk.must_equal("id")
- end
-
- it "should parse column type in show create table" do
- @table.columns["username"][:type].must_equal("varchar(255)")
- end
-
- it "should parse column metadata" do
- @table.columns["username"][:metadata].must_equal("DEFAULT NULL")
- end
-
- it "should parse columns with only a name and type" do
- @table.columns.keys.include?(:description).must_equal true
- end
-
- it "should parse indices in show create table" do
- @table.
- indices["index_users_on_username_and_created_at"][:metadata].
- must_equal("(`username`,`created_at`)")
- end
-
- it "should parse indices in show create table" do
- @table.
- indices["index_users_on_reference"][:metadata].
- must_equal("(`reference`)")
- end
- end
- end
end
Please sign in to comment.
Something went wrong with that request. Please try again.