Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Comparing changes

Choose two branches to see what's changed or to start a new pull request. If you need to, you can also compare across forks.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also compare across forks.
base fork: rails/rails
...
head fork: rails/rails
  • 20 commits
  • 11 files changed
  • 6 commit comments
  • 1 contributor
Commits on Mar 22, 2013
@tenderlove tenderlove remove knowledge of SQL from the column definition object 69ef76a
@tenderlove tenderlove stop depending on sql_type in pg 4b4c8bd
@tenderlove tenderlove decouple column definition from the database connection cd07f19
@tenderlove tenderlove remove to_sql from TableDefinition 1c9f7fa
@tenderlove tenderlove mostly decouple TableDefinition from the database connection a80bcc3
@tenderlove tenderlove push SQL generation inside the schema creation object 14d7dc0
@tenderlove tenderlove factory methods should not alter object state a724096
@tenderlove tenderlove keep ivars private, do not manipulate them outside their owner object c5e03e8
@tenderlove tenderlove @columns list is no longer necessary b8a533d
@tenderlove tenderlove push column initialization down to the factory method d43edf6
@tenderlove tenderlove there is no reason to check for an already defined column f84cf41
@tenderlove tenderlove push alter table add column sql in to the schema modification visitor f20b2f4
@tenderlove tenderlove add a pg visitor for dealing with schema modification 6b7fdf3
@tenderlove tenderlove pull add_column_options! off the pg connection class 072dbbf
@tenderlove tenderlove allow multiple add columns 739a720
@tenderlove tenderlove push the mysql add_column up to the abstract adapter 2ac300b
Commits on Mar 23, 2013
@tenderlove tenderlove separate primary key from column type d25e407
@tenderlove tenderlove add uuid primary key support bc8ebef
@tenderlove tenderlove squelch an unused variable warning db3a6e6
@tenderlove tenderlove Merge branch 'schema'
* schema:
  add uuid primary key support
  separate primary key from column type
  push the mysql add_column up to the abstract adapter
  allow multiple add columns
  pull add_column_options! off the pg connection class
  add a pg visitor for dealing with schema modification
  push alter table add column sql in to the schema modification visitor
  there is no reason to check for an already defined column
  push column initialization down to the factory method
  @columns list is no longer necessary
  keep ivars private, do not manipulate them outside their owner object
  factory methods should not alter object state
  push SQL generation inside the schema creation object
  mostly decouple TableDefinition from the database connection
  remove to_sql from TableDefinition
  decouple column definition from the database connection
  stop depending on sql_type in pg
  remove knowledge of SQL from the column definition object
7a872e9
View
108 activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -15,34 +15,14 @@ class IndexDefinition < Struct.new(:table, :name, :unique, :columns, :lengths, :
# are typically created by methods in TableDefinition, and added to the
# +columns+ attribute of said TableDefinition object, in order to be used
# for generating a number of table creation or table changing SQL statements.
- class ColumnDefinition < Struct.new(:base, :name, :type, :limit, :precision, :scale, :default, :null) #:nodoc:
+ class ColumnDefinition < Struct.new(:name, :type, :limit, :precision, :scale, :default, :null, :first, :after, :primary_key) #:nodoc:
def string_to_binary(value)
value
end
- def sql_type
- base.type_to_sql(type.to_sym, limit, precision, scale)
- end
-
def primary_key?
- type.to_sym == :primary_key
- end
-
- def to_sql
- column_sql = "#{base.quote_column_name(name)} #{sql_type}"
- column_options = {}
- column_options[:null] = null unless null.nil?
- column_options[:default] = default unless default.nil?
- column_options[:column] = self
- add_column_options!(column_sql, column_options) unless primary_key?
- column_sql
+ primary_key || type.to_sym == :primary_key
end
-
- private
-
- def add_column_options!(sql, options)
- base.add_column_options!(sql, options)
- end
end
# Represents the schema of an SQL table in an abstract way. This class
@@ -68,19 +48,25 @@ def add_column_options!(sql, options)
class TableDefinition
# An array of ColumnDefinition objects, representing the column changes
# that have been defined.
- attr_accessor :columns, :indexes
+ attr_accessor :indexes
+ attr_reader :name, :temporary, :options
- def initialize(base)
- @columns = []
+ def initialize(types, name, temporary, options)
@columns_hash = {}
@indexes = {}
- @base = base
+ @native = types
+ @temporary = temporary
+ @options = options
+ @name = name
end
+ def columns; @columns_hash.values; end
+
# Appends a primary key definition to the table definition.
# Can be called multiple times, but this is probably not a good idea.
- def primary_key(name)
- column(name, :primary_key)
+ def primary_key(name, type = :primary_key, options = {})
+ options[:primary_key] = true
+ column(name, type, options)
end
# Returns a ColumnDefinition for the column with name +name+.
@@ -233,20 +219,14 @@ def column(name, type, options = {})
raise ArgumentError, "you can't redefine the primary key column '#{name}'. To define a custom primary key, pass { id: false } to create_table."
end
- column = self[name] || new_column_definition(@base, name, type)
-
- limit = options.fetch(:limit) do
- native[type][:limit] if native[type].is_a?(Hash)
- end
-
- column.limit = limit
- column.precision = options[:precision]
- column.scale = options[:scale]
- column.default = options[:default]
- column.null = options[:null]
+ @columns_hash[name] = new_column_definition(name, type, options)
self
end
+ def remove_column(name)
+ @columns_hash.delete name.to_s
+ end
+
[:string, :text, :integer, :float, :decimal, :datetime, :timestamp, :time, :date, :binary, :boolean].each do |column_type|
define_method column_type do |*args|
options = args.extract_options!
@@ -283,23 +263,26 @@ def references(*args)
end
alias :belongs_to :references
- # Returns a String whose contents are the column definitions
- # concatenated together. This string can then be prepended and appended to
- # to generate the final SQL to create the table.
- def to_sql
- columns.map { |c| c.to_sql } * ', '
- end
+ def new_column_definition(name, type, options) # :nodoc:
+ column = create_column_definition name, type
+ limit = options.fetch(:limit) do
+ native[type][:limit] if native[type].is_a?(Hash)
+ end
- private
- def create_column_definition(base, name, type)
- ColumnDefinition.new base, name, type
+ column.limit = limit
+ column.precision = options[:precision]
+ column.scale = options[:scale]
+ column.default = options[:default]
+ column.null = options[:null]
+ column.first = options[:first]
+ column.after = options[:after]
+ column.primary_key = type == :primary_key || options[:primary_key]
+ column
end
- def new_column_definition(base, name, type)
- definition = create_column_definition base, name, type
- @columns << definition
- @columns_hash[name] = definition
- definition
+ private
+ def create_column_definition(name, type)
+ ColumnDefinition.new name, type
end
def primary_key_column_name
@@ -308,7 +291,24 @@ def primary_key_column_name
end
def native
- @base.native_database_types
+ @native
+ end
+ end
+
+ class AlterTable # :nodoc:
+ attr_reader :adds
+
+ def initialize(td)
+ @td = td
+ @adds = []
+ end
+
+ def name; @td.name; end
+
+ def add_column(name, type, options)
+ name = name.to_s
+ type = type.to_sym
+ @adds << @td.new_column_definition(name, type, options)
end
end
View
24 activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -171,14 +171,14 @@ def column_exists?(table_name, column_name, type = nil, options = {})
#
# See also TableDefinition#column for details on how to create columns.
def create_table(table_name, options = {})
- td = create_table_definition
+ td = create_table_definition table_name, options[:temporary], options[:options]
unless options[:id] == false
pk = options.fetch(:primary_key) {
Base.get_primary_key table_name.to_s.singularize
}
- td.primary_key pk
+ td.primary_key pk, options.fetch(:id, :primary_key), options
end
yield td if block_given?
@@ -187,11 +187,7 @@ def create_table(table_name, options = {})
drop_table(table_name, options)
end
- create_sql = "CREATE#{' TEMPORARY' if options[:temporary]} TABLE "
- create_sql << "#{quote_table_name(table_name)} ("
- create_sql << td.to_sql
- create_sql << ") #{options[:options]}"
- execute create_sql
+ execute schema_creation.accept td
td.indexes.each_pair { |c,o| add_index table_name, c, o }
end
@@ -359,9 +355,9 @@ def drop_table(table_name, options = {})
# Adds a new column to the named table.
# See TableDefinition#column for details of the options you can use.
def add_column(table_name, column_name, type, options = {})
- add_column_sql = "ALTER TABLE #{quote_table_name(table_name)} ADD #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
- add_column_options!(add_column_sql, options)
- execute(add_column_sql)
+ at = create_alter_table table_name
+ at.add_column(column_name, type, options)
+ execute schema_creation.accept at
end
# Removes the given columns from the table definition.
@@ -829,8 +825,12 @@ def rename_column_indexes(table_name, column_name, new_column_name)
end
private
- def create_table_definition
- TableDefinition.new(self)
+ def create_table_definition(name, temporary, options)
+ TableDefinition.new native_database_types, name, temporary, options
+ end
+
+ def create_alter_table(name)
+ AlterTable.new create_table_definition(name, false, {})
end
def update_table_definition(table_name, base)
View
69 activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -100,6 +100,75 @@ def initialize(connection, logger = nil, pool = nil) #:nodoc:
@visitor = nil
end
+ class SchemaCreation
+ def initialize(conn)
+ @conn = conn
+ @cache = {}
+ end
+
+ def accept(o)
+ m = @cache[o.class] ||= "visit_#{o.class.name.split('::').last}"
+ send m, o
+ end
+
+ private
+
+ def visit_AlterTable(o)
+ sql = "ALTER TABLE #{quote_table_name(o.name)} "
+ sql << o.adds.map { |col| visit_AddColumn col }.join(' ')
+ end
+
+ def visit_AddColumn(o)
+ sql_type = type_to_sql(o.type.to_sym, o.limit, o.precision, o.scale)
+ sql = "ADD #{quote_column_name(o.name)} #{sql_type}"
+ add_column_options!(sql, column_options(o))
+ end
+
+ def visit_ColumnDefinition(o)
+ sql_type = type_to_sql(o.type.to_sym, o.limit, o.precision, o.scale)
+ column_sql = "#{quote_column_name(o.name)} #{sql_type}"
+ add_column_options!(column_sql, column_options(o)) unless o.primary_key?
+ column_sql
+ end
+
+ def visit_TableDefinition(o)
+ create_sql = "CREATE#{' TEMPORARY' if o.temporary} TABLE "
+ create_sql << "#{quote_table_name(o.name)} ("
+ create_sql << o.columns.map { |c| accept c }.join(', ')
+ create_sql << ") #{o.options}"
+ create_sql
+ end
+
+ def column_options(o)
+ column_options = {}
+ column_options[:null] = o.null unless o.null.nil?
+ column_options[:default] = o.default unless o.default.nil?
+ column_options[:column] = o
+ column_options
+ end
+
+ def quote_column_name(name)
+ @conn.quote_column_name name
+ end
+
+ def quote_table_name(name)
+ @conn.quote_table_name name
+ end
+
+ def type_to_sql(type, limit, precision, scale)
+ @conn.type_to_sql type.to_sym, limit, precision, scale
+ end
+
+ def add_column_options!(column_sql, column_options)
+ @conn.add_column_options! column_sql, column_options
+ column_sql
+ end
+ end
+
+ def schema_creation
+ SchemaCreation.new self
+ end
+
def lease
synchronize do
unless in_use
View
25 activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -3,6 +3,27 @@
module ActiveRecord
module ConnectionAdapters
class AbstractMysqlAdapter < AbstractAdapter
+ class SchemaCreation < AbstractAdapter::SchemaCreation
+ private
+
+ def visit_AddColumn(o)
+ add_column_position!(super, o)
+ end
+
+ def add_column_position!(sql, column)
+ if column.first
+ sql << " FIRST"
+ elsif column.after
+ sql << " AFTER #{quote_column_name(column.after)}"
+ end
+ sql
+ end
+ end
+
+ def schema_creation
+ SchemaCreation.new self
+ end
+
class Column < ConnectionAdapters::Column # :nodoc:
attr_reader :collation, :strict
@@ -459,10 +480,6 @@ def rename_table(table_name, new_name)
rename_table_indexes(table_name, new_name)
end
- def add_column(table_name, column_name, type, options = {})
- execute("ALTER TABLE #{quote_table_name(table_name)} #{add_column_sql(table_name, column_name, type, options)}")
- end
-
def change_column_default(table_name, column_name, default)
column = column_for(table_name, column_name)
change_column table_name, column_name, column.sql_type, :default => default
View
14 activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
@@ -18,10 +18,12 @@ def unescape_bytea(value)
def quote(value, column = nil) #:nodoc:
return super unless column
+ sql_type = type_to_sql(column.type, column.limit, column.precision, column.scale)
+
case value
when Range
- if /range$/ =~ column.sql_type
- "'#{PostgreSQLColumn.range_to_string(value)}'::#{column.sql_type}"
+ if /range$/ =~ sql_type
+ "'#{PostgreSQLColumn.range_to_string(value)}'::#{sql_type}"
else
super
end
@@ -32,13 +34,13 @@ def quote(value, column = nil) #:nodoc:
super
end
when Hash
- case column.sql_type
+ case sql_type
when 'hstore' then super(PostgreSQLColumn.hstore_to_string(value), column)
when 'json' then super(PostgreSQLColumn.json_to_string(value), column)
else super
end
when IPAddr
- case column.sql_type
+ case sql_type
when 'inet', 'cidr' then super(PostgreSQLColumn.cidr_to_string(value), column)
else super
end
@@ -51,14 +53,14 @@ def quote(value, column = nil) #:nodoc:
super
end
when Numeric
- if column.sql_type == 'money' || [:string, :text].include?(column.type)
+ if sql_type == 'money' || [:string, :text].include?(column.type)
# Not truly string input, so doesn't require (or allow) escape string syntax.
"'#{value}'"
else
super
end
when String
- case column.sql_type
+ case sql_type
when 'bytea' then "'#{escape_bytea(value)}'"
when 'xml' then "xml '#{quote_string(value)}'"
when /^bit/
View
41 activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -1,6 +1,42 @@
module ActiveRecord
module ConnectionAdapters
class PostgreSQLAdapter < AbstractAdapter
+ class SchemaCreation < AbstractAdapter::SchemaCreation
+ private
+
+ def visit_AddColumn(o)
+ sql_type = type_to_sql(o.type.to_sym, o.limit, o.precision, o.scale)
+ sql = "ADD COLUMN #{quote_column_name(o.name)} #{sql_type}"
+ add_column_options!(sql, column_options(o))
+ end
+
+ def visit_ColumnDefinition(o)
+ sql = super
+ if o.primary_key? && o.type == :uuid
+ sql << " PRIMARY KEY "
+ add_column_options!(sql, column_options(o))
+ end
+ sql
+ end
+
+ def add_column_options!(sql, options)
+ if options[:array] || options[:column].try(:array)
+ sql << '[]'
+ end
+
+ column = options.fetch(:column) { return super }
+ if column.type == :uuid && options[:default] =~ /\(\)/
+ sql << " DEFAULT #{options[:default]}"
+ else
+ super
+ end
+ end
+ end
+
+ def schema_creation
+ SchemaCreation.new self
+ end
+
module SchemaStatements
# Drops the database specified on the +name+ attribute
# and creates it again using the provided +options+.
@@ -337,10 +373,7 @@ def rename_table(table_name, new_name)
# See TableDefinition#column for details of the options you can use.
def add_column(table_name, column_name, type, options = {})
clear_cache!
- add_column_sql = "ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
- add_column_options!(add_column_sql, options)
-
- execute add_column_sql
+ super
end
# Changes the column of a table.
View
28 activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -330,6 +330,13 @@ def json(name, options = {})
class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition
include ColumnMethods
+ def primary_key(name, type = :primary_key, options = {})
+ return super unless type == :uuid
+ options[:default] ||= 'uuid_generate_v4()'
+ options[:primary_key] = true
+ column name, type, options
+ end
+
def column(name, type = nil, options = {})
super
column = self[name]
@@ -344,8 +351,8 @@ def xml(options = {})
private
- def create_column_definition(base, name, type)
- ColumnDefinition.new base, name, type
+ def create_column_definition(name, type)
+ ColumnDefinition.new name, type
end
end
@@ -627,19 +634,6 @@ def table_alias_length
@table_alias_length ||= query('SHOW max_identifier_length', 'SCHEMA')[0][0].to_i
end
- def add_column_options!(sql, options)
- if options[:array] || options[:column].try(:array)
- sql << '[]'
- end
-
- column = options.fetch(:column) { return super }
- if column.type == :uuid && options[:default] =~ /\(\)/
- sql << " DEFAULT #{options[:default]}"
- else
- super
- end
- end
-
# Set the authorized user for this session
def session_auth=(user)
clear_cache!
@@ -900,8 +894,8 @@ def extract_table_ref_from_insert_sql(sql)
$1.strip if $1
end
- def create_table_definition
- TableDefinition.new(self)
+ def create_table_definition(name, temporary, options)
+ TableDefinition.new native_database_types, name, temporary, options
end
def update_table_definition(table_name, base)
View
2  activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -458,7 +458,7 @@ def add_column(table_name, column_name, type, options = {}) #:nodoc:
def remove_column(table_name, column_name, type = nil, options = {}) #:nodoc:
alter_table(table_name) do |definition|
- definition.columns.delete(definition[column_name])
+ definition.remove_column column_name
end
end
View
10 activerecord/test/cases/adapters/postgresql/uuid_test.rb
@@ -35,6 +35,16 @@ def teardown
@connection.execute 'drop table if exists pg_uuids'
end
+ def test_id_is_uuid
+ assert_equal :uuid, UUID.columns_hash['id'].type
+ assert UUID.primary_key
+ end
+
+ def test_id_has_a_default
+ u = UUID.create
+ assert_not_nil u.id
+ end
+
def test_auto_create_uuid
u = UUID.create
u.reload
View
13 activerecord/test/cases/column_definition_test.rb
@@ -8,6 +8,7 @@ def setup
def @adapter.native_database_types
{:string => "varchar"}
end
+ @viz = @adapter.schema_creation
end
def test_can_set_coder
@@ -35,25 +36,25 @@ def test_type_case_coded_column
def test_should_not_include_default_clause_when_default_is_null
column = Column.new("title", nil, "varchar(20)")
column_def = ColumnDefinition.new(
- @adapter, column.name, "string",
+ column.name, "string",
column.limit, column.precision, column.scale, column.default, column.null)
- assert_equal "title varchar(20)", column_def.to_sql
+ assert_equal "title varchar(20)", @viz.accept(column_def)
end
def test_should_include_default_clause_when_default_is_present
column = Column.new("title", "Hello", "varchar(20)")
column_def = ColumnDefinition.new(
- @adapter, column.name, "string",
+ column.name, "string",
column.limit, column.precision, column.scale, column.default, column.null)
- assert_equal %Q{title varchar(20) DEFAULT 'Hello'}, column_def.to_sql
+ assert_equal %Q{title varchar(20) DEFAULT 'Hello'}, @viz.accept(column_def)
end
def test_should_specify_not_null_if_null_option_is_false
column = Column.new("title", "Hello", "varchar(20)", false)
column_def = ColumnDefinition.new(
- @adapter, column.name, "string",
+ column.name, "string",
column.limit, column.precision, column.scale, column.default, column.null)
- assert_equal %Q{title varchar(20) DEFAULT 'Hello' NOT NULL}, column_def.to_sql
+ assert_equal %Q{title varchar(20) DEFAULT 'Hello' NOT NULL}, @viz.accept(column_def)
end
if current_adapter?(:MysqlAdapter)
View
1  activerecord/test/cases/counter_cache_test.rb
@@ -117,6 +117,7 @@ class ::SpecialReply < ::Reply
test "update other counters on parent destroy" do
david, joanna = dog_lovers(:david, :joanna)
+ joanna = joanna # squelch a warning
assert_difference 'joanna.reload.dogs_count', -1 do
david.destroy

Showing you all comments on commits in this comparison.

@rubys
uninitialized constant ActiveRecord::ConnectionAdapters::SchemaStatements::AlterTable

http://intertwingly.net/projects/AWDwR4/checkdepot/section-10.1.html

@rafaelfranca

Should be fixed by 6ff36de

@natebird

Bravo @tenderlove. Bravo!

@kodemunki

Excellent. Thanks @tenderlove!

@nertzy

This modifies the options hash! See #10572

Something went wrong with that request. Please try again.