diff options
Diffstat (limited to 'activerecord')
17 files changed, 205 insertions, 46 deletions
diff --git a/activerecord/activerecord.gemspec b/activerecord/activerecord.gemspec index c91a6ccd35..2de81c31a3 100644 --- a/activerecord/activerecord.gemspec +++ b/activerecord/activerecord.gemspec @@ -21,6 +21,6 @@ Gem::Specification.new do |s| s.add_dependency('activesupport', version) s.add_dependency('activemodel', version) - s.add_dependency('arel', '~> 2.2.0') + s.add_dependency('arel', '~> 2.2.1') s.add_dependency('tzinfo', '~> 0.3.29') end diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index d63b9e3f24..511d402ee5 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -42,7 +42,7 @@ module ActiveRecord autoload :ActiveRecordError, 'active_record/errors' autoload :ConnectionNotEstablished, 'active_record/errors' autoload :ConnectionAdapters, 'active_record/connection_adapters/abstract_adapter' - + autoload :Aggregations autoload :Associations autoload :AttributeMethods @@ -72,6 +72,7 @@ module ActiveRecord autoload :Persistence autoload :QueryCache autoload :Reflection + autoload :Result autoload :Schema autoload :SchemaDumper autoload :Serialization diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 2605a54cb6..8d755b6848 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1087,7 +1087,8 @@ module ActiveRecord # # [:finder_sql] # Specify a complete SQL statement to fetch the association. This is a good way to go for complex - # associations that depend on multiple tables. Note: When this option is used, +find_in_collection+ + # associations that depend on multiple tables. May be supplied as a string or a proc where interpolation is + # required. Note: When this option is used, +find_in_collection+ # is _not_ added. # [:counter_sql] # Specify a complete SQL statement to fetch the size of the association. If <tt>:finder_sql</tt> is @@ -1162,11 +1163,14 @@ module ActiveRecord # has_many :tags, :as => :taggable # has_many :reports, :readonly => true # has_many :subscribers, :through => :subscriptions, :source => :user - # has_many :subscribers, :class_name => "Person", :finder_sql => - # 'SELECT DISTINCT people.* ' + - # 'FROM people p, post_subscriptions ps ' + - # 'WHERE ps.post_id = #{id} AND ps.person_id = p.id ' + - # 'ORDER BY p.first_name' + # has_many :subscribers, :class_name => "Person", :finder_sql => Proc.new { + # %Q{ + # SELECT DISTINCT people.* + # FROM people p, post_subscriptions ps + # WHERE ps.post_id = #{id} AND ps.person_id = p.id + # ORDER BY p.first_name + # } + # } def has_many(name, options = {}, &extension) Builder::HasMany.build(self, name, options, &extension) end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index bb519c5703..d1e3ff8e38 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -151,20 +151,20 @@ module ActiveRecord reset end + def interpolate(sql, record = nil) + if sql.respond_to?(:to_proc) + owner.send(:instance_exec, record, &sql) + else + sql + end + end + private def find_target? !loaded? && (!owner.new_record? || foreign_key_present?) && klass end - def interpolate(sql, record = nil) - if sql.respond_to?(:to_proc) - owner.send(:instance_exec, record, &sql) - else - sql - end - end - def creation_attributes attributes = {} diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index 198ad06360..2ee5dbbd70 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -2,6 +2,11 @@ module ActiveRecord # = Active Record Belongs To Polymorphic Association module Associations class BelongsToPolymorphicAssociation < BelongsToAssociation #:nodoc: + def klass + type = owner[reflection.foreign_type] + type.presence && type.constantize + end + private def replace_keys(record) @@ -17,11 +22,6 @@ module ActiveRecord reflection.polymorphic_inverse_of(record.class) end - def klass - type = owner[reflection.foreign_type] - type.presence && type.constantize - end - def raise_on_type_mismatch(record) # A polymorphic association cannot have a type mismatch, by definition end diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 504f25271c..6c878f0f00 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -188,13 +188,12 @@ module ActiveRecord association = join_part.instantiate(row) unless row[join_part.aliased_primary_key].nil? set_target_and_inverse(join_part, association, record) else - return if row[join_part.aliased_primary_key].nil? - association = join_part.instantiate(row) + association = join_part.instantiate(row) unless row[join_part.aliased_primary_key].nil? case macro when :has_many, :has_and_belongs_to_many other = record.association(join_part.reflection.name) other.loaded! - other.target.push(association) + other.target.push(association) if association other.set_inverse_instance(association) when :belongs_to set_target_and_inverse(join_part, association, record) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 59977280b3..c76f98d6a0 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -178,7 +178,7 @@ module ActiveRecord #:nodoc: # <tt>Person.find_all_by_last_name(last_name)</tt>. # # It's possible to add an exclamation point (!) on the end of the dynamic finders to get them to raise an - # <tt>ActiveRecord::RecordNotFound</tt> error if they do not return any records, + # <tt>ActiveRecord::RecordNotFound</tt> error if they do not return any records, # like <tt>Person.find_by_last_name!</tt>. # # It's also possible to use multiple attributes in the same find by separating them with "_and_". @@ -2163,4 +2163,5 @@ MSG end end +require 'active_record/connection_adapters/abstract/connection_specification' ActiveSupport.run_load_hooks(:active_record, ActiveRecord::Base) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 2ae655e68d..dc4a53034b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -306,6 +306,16 @@ module ActiveRecord end end + # The default strategy for an UPDATE with joins is to use a subquery. This doesn't work + # on mysql (even when aliasing the tables), but mysql allows using JOIN directly in + # an UPDATE statement, so in the mysql adapters we redefine this to do that. + def join_to_update(update, select) #:nodoc: + subselect = select.clone + subselect.projections = [update.key] + + update.where update.key.in(subselect) + end + protected # Returns an array of record hashes with the column names as keys and # column values as values. diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 077cf7df1b..60e2f811a2 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -4,20 +4,30 @@ require 'bigdecimal/util' require 'active_support/core_ext/benchmark' require 'active_support/deprecation' -# TODO: Autoload these files -require 'active_record/connection_adapters/column' -require 'active_record/connection_adapters/abstract/schema_definitions' -require 'active_record/connection_adapters/abstract/schema_statements' -require 'active_record/connection_adapters/abstract/database_statements' -require 'active_record/connection_adapters/abstract/quoting' -require 'active_record/connection_adapters/abstract/connection_pool' -require 'active_record/connection_adapters/abstract/connection_specification' -require 'active_record/connection_adapters/abstract/query_cache' -require 'active_record/connection_adapters/abstract/database_limits' -require 'active_record/result' - module ActiveRecord module ConnectionAdapters # :nodoc: + extend ActiveSupport::Autoload + + autoload :Column + + autoload_under 'abstract' do + autoload :IndexDefinition, 'active_record/connection_adapters/abstract/schema_definitions' + autoload :ColumnDefinition, 'active_record/connection_adapters/abstract/schema_definitions' + autoload :TableDefinition, 'active_record/connection_adapters/abstract/schema_definitions' + + autoload :SchemaStatements + autoload :DatabaseStatements + autoload :DatabaseLimits + autoload :Quoting + + autoload :ConnectionPool + autoload :ConnectionHandler, 'active_record/connection_adapters/abstract/connection_pool' + autoload :ConnectionManagement, 'active_record/connection_adapters/abstract/connection_pool' + autoload :ConnectionSpecification + + autoload :QueryCache + end + # Active Record supports multiple database systems. AbstractAdapter and # related classes form the abstraction layer which makes this possible. # An AbstractAdapter represents a connection to a database, and provides an diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 18fdfa29ec..ef51f5ebca 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -169,7 +169,7 @@ module ActiveRecord end def quote_column_name(name) #:nodoc: - @quoted_column_names[name] ||= "`#{name}`" + @quoted_column_names[name] ||= "`#{name.to_s.gsub('`', '``')}`" end def quote_table_name(name) #:nodoc: @@ -577,6 +577,27 @@ module ActiveRecord where_sql end + # In the simple case, MySQL allows us to place JOINs directly into the UPDATE + # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support + # these, we must use a subquery. However, MySQL is too stupid to create a + # temporary table for this automatically, so we have to give it some prompting + # in the form of a subsubquery. Ugh! + def join_to_update(update, select) #:nodoc: + if select.limit || select.offset || select.orders.any? + subsubselect = select.clone + subsubselect.projections = [update.key] + + subselect = Arel::SelectManager.new(select.engine) + subselect.project Arel.sql(update.key.name) + subselect.from subsubselect.as('__active_record_temp') + + update.where update.key.in(subselect) + else + update.table select.source + update.wheres = select.constraints + end + end + protected def quoted_columns_for_index(column_names, options = {}) length = options[:length] if options.is_a?(Hash) diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 14b950dbb0..b844e5ab10 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -250,7 +250,7 @@ module ActiveRecord end def quote_column_name(name) #:nodoc: - @quoted_column_names[name] ||= "`#{name}`" + @quoted_column_names[name] ||= "`#{name.to_s.gsub('`', '``')}`" end def quote_table_name(name) #:nodoc: @@ -491,6 +491,27 @@ module ActiveRecord execute("RELEASE SAVEPOINT #{current_savepoint_name}") end + # In the simple case, MySQL allows us to place JOINs directly into the UPDATE + # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support + # these, we must use a subquery. However, MySQL is too stupid to create a + # temporary table for this automatically, so we have to give it some prompting + # in the form of a subsubquery. Ugh! + def join_to_update(update, select) #:nodoc: + if select.limit || select.offset || select.orders.any? + subsubselect = select.clone + subsubselect.projections = [update.key] + + subselect = Arel::SelectManager.new(select.engine) + subselect.project Arel.sql(update.key.name) + subselect.from subsubselect.as('__active_record_temp') + + update.where update.key.in(subselect) + else + update.table select.source + update.wheres = select.constraints + end + end + # SCHEMA STATEMENTS ======================================== def structure_dump #:nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 486efc5ba0..da86957028 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -148,7 +148,7 @@ module ActiveRecord end def quote_column_name(name) #:nodoc: - %Q("#{name}") + %Q("#{name.to_s.gsub('"', '""')}") end # Quote date/time values for use in SQL input. Includes microseconds diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 7e59eb4584..15fd1a58c8 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -216,14 +216,20 @@ module ActiveRecord if conditions || options.present? where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates) else - stmt = arel.compile_update(Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates))) + stmt = Arel::UpdateManager.new(arel.engine) - if limit = arel.limit - stmt.take limit + stmt.set Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates)) + stmt.table(table) + stmt.key = table[primary_key] + + if joins_values.any? + @klass.connection.join_to_update(stmt, arel) + else + stmt.take(arel.limit) + stmt.order(*arel.orders) + stmt.wheres = arel.constraints end - stmt.order(*arel.orders) - stmt.key = table[primary_key] @klass.connection.update stmt, 'SQL', bind_values end end diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 2814771002..7e8ddd1b5d 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -19,7 +19,7 @@ module ActiveRecord case value when ActiveRecord::Relation - value.select_values = [value.klass.arel_table['id']] if value.select_values.empty? + value = value.select(value.klass.arel_table[value.klass.primary_key]) if value.select_values.empty? attribute.in(value.arel.ast) when Array, ActiveRecord::Associations::CollectionProxy values = value.to_a.map { |x| diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 0b1ba31ac2..5f2328ff95 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -813,4 +813,12 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert !c.save end end + + def test_preloading_empty_through_association_via_joins + person = Person.create!(:first_name => "Gaga") + person = Person.where(:id => person.id).where('readers.id = 1 or 1=1').includes(:posts).to_a.first + + assert person.posts.loaded?, 'person.posts should be loaded' + assert_equal [], person.posts + end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index b8ebabfe70..fe46c00b47 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -67,6 +67,23 @@ end class BasicsTest < ActiveRecord::TestCase fixtures :topics, :companies, :developers, :projects, :computers, :accounts, :minimalistics, 'warehouse-things', :authors, :categorizations, :categories, :posts + def test_column_names_are_escaped + conn = ActiveRecord::Base.connection + classname = conn.class.name[/[^:]*$/] + badchar = { + 'SQLite3Adapter' => '"', + 'MysqlAdapter' => '`', + 'Mysql2Adapter' => '`', + 'PostgreSQLAdapter' => '"', + 'OracleAdapter' => '"', + }.fetch(classname) { + raise "need a bad char for #{classname}" + } + + quoted = conn.quote_column_name "foo#{badchar}bar" + assert_equal("#{badchar}foo#{badchar * 2}bar#{badchar}", quoted) + end + def test_columns_should_obey_set_primary_key pk = Subscriber.columns.find { |x| x.name == 'nick' } assert pk.primary, 'nick should be primary key' diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 821da91f0a..615551a279 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -540,6 +540,29 @@ class RelationTest < ActiveRecord::TestCase } end + def test_find_all_using_where_with_relation_and_alternate_primary_key + cool_first = minivans(:cool_first) + # switching the lines below would succeed in current rails + # assert_queries(2) { + assert_queries(1) { + relation = Minivan.where(:minivan_id => Minivan.where(:name => cool_first.name)) + assert_equal [cool_first], relation.all + } + end + + def test_find_all_using_where_with_relation_does_not_alter_select_values + david = authors(:david) + + subquery = Author.where(:id => david.id) + + assert_queries(1) { + relation = Author.where(:id => subquery) + assert_equal [david], relation.all + } + + assert_equal 0, subquery.select_values.size + end + def test_find_all_using_where_with_relation_with_joins david = authors(:david) assert_queries(1) { @@ -965,4 +988,42 @@ class RelationTest < ActiveRecord::TestCase def test_ordering_with_extra_spaces assert_equal authors(:david), Author.order('id DESC , name DESC').last end + + def test_update_all_with_joins + comments = Comment.joins(:post).where('posts.id' => posts(:welcome).id) + count = comments.count + + assert_equal count, comments.update_all(:post_id => posts(:thinking).id) + assert_equal posts(:thinking), comments(:greetings).post + end + + def test_update_all_with_joins_and_limit + comments = Comment.joins(:post).where('posts.id' => posts(:welcome).id).limit(1) + assert_equal 1, comments.update_all(:post_id => posts(:thinking).id) + end + + def test_update_all_with_joins_and_limit_and_order + comments = Comment.joins(:post).where('posts.id' => posts(:welcome).id).order('comments.id').limit(1) + assert_equal 1, comments.update_all(:post_id => posts(:thinking).id) + assert_equal posts(:thinking), comments(:greetings).post + assert_equal posts(:welcome), comments(:more_greetings).post + end + + def test_update_all_with_joins_and_offset + all_comments = Comment.joins(:post).where('posts.id' => posts(:welcome).id) + count = all_comments.count + comments = all_comments.offset(1) + + assert_equal count - 1, comments.update_all(:post_id => posts(:thinking).id) + end + + def test_update_all_with_joins_and_offset_and_order + all_comments = Comment.joins(:post).where('posts.id' => posts(:welcome).id).order('posts.id') + count = all_comments.count + comments = all_comments.offset(1) + + assert_equal count - 1, comments.update_all(:post_id => posts(:thinking).id) + assert_equal posts(:thinking), comments(:more_greetings).post + assert_equal posts(:welcome), comments(:greetings).post + end end |