Merge branch 'master' into adequaterecord

* master: (28 commits)
  move AR length validation tests into separate test-case.
  No need for trailing slash on migration path.
  reset `@arel` when modifying a Relation in place.
  PostgreSQL Timestamps always map to `:datetime`.
  [ci skip] Improve formatting and yml
  Fix a typo in the doc of forty_two AR FinderMethod
  Improve readability of contributing to rails guide. [ci skip]
  Precompile the image we're referencing, too.
  `ActiveRecord::Base.no_touching` no longer triggers callbacks or start empty transactions.
  Fixed an issue with migrating legacy json cookies.
  Correct comment [ci skip]
  Perfer to define methods instead of calling test
  Fix syntax error
  Add CHANGELOG entry for #14757 [ci skip]
  Fix run-on sentences and improve grammar [skip ci]
  Add test for using ActionView::Helpers::FormHelper.label with block and html
  select! renamed to avoid name collision Array#select!
  Rearrange deck chairs on the titanic. Organize connection handling test cases.
  Change favicon_link_tag helper mimetype from image/vnd.microsoft.icon to image/x-icon.
  ActionController::Renderers documentation fix
  ...

9 files changed, 33 insertions, 16 deletions

diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb
index 83c69586e6..bf461070e0 100644
--- a/activerecord/lib/active_record/associations/preloader/association.rb
+++ b/activerecord/lib/active_record/associations/preloader/association.rb
@@ -119,7 +119,7 @@ module ActiveRecord
           scope.references_values = Array(values[:references]) + Array(preload_values[:references])
           scope.bind_values       = (reflection_binds + preload_binds)
 
-          scope.select!   preload_values[:select] || values[:select] || table[Arel.star]
+          scope._select!   preload_values[:select] || values[:select] || table[Arel.star]
           scope.includes! preload_values[:includes] || values[:includes]
 
           if preload_values.key? :order
diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb
index 70e97432e4..1fed7f74e7 100644
--- a/activerecord/lib/active_record/associations/preloader/through_association.rb
+++ b/activerecord/lib/active_record/associations/preloader/through_association.rb
@@ -84,7 +84,7 @@ module ActiveRecord
             end
 
             scope.references! reflection_scope.values[:references]
-            scope.order! reflection_scope.values[:order] if scope.eager_loading?
+            scope = scope.order reflection_scope.values[:order] if scope.eager_loading?
           end
 
           scope
diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb
index ba7d2a3782..f8a85b8a6f 100644
--- a/activerecord/lib/active_record/associations/through_association.rb
+++ b/activerecord/lib/active_record/associations/through_association.rb
@@ -14,9 +14,11 @@ module ActiveRecord
         def target_scope
           scope = super
           chain.drop(1).each do |reflection|
+            relation = reflection.klass.all
+            relation.merge!(reflection.scope) if reflection.scope
+
             scope.merge!(
-              reflection.klass.all.
-                except(:select, :create_with, :includes, :preload, :joins, :eager_load)
+              relation.except(:select, :create_with, :includes, :preload, :joins, :eager_load)
             )
           end
           scope
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 1d47cba234..db4d5f0129 100644
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -296,7 +296,6 @@ module ActiveRecord #:nodoc:
 
     include Core
     include Persistence
-    include NoTouching
     include ReadonlyAttributes
     include ModelSchema
     include Inheritance
@@ -318,6 +317,7 @@ module ActiveRecord #:nodoc:
     include NestedAttributes
     include Aggregations
     include Transactions
+    include NoTouching
     include Reflection
     include Serialization
     include Store
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
index 9e898015a6..540b3694b5 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb
@@ -209,12 +209,7 @@ This is not reliable and will be removed in the future.
         class Timestamp < Type
           def type; :timestamp; end
           def simplified_type(sql_type)
-            case sql_type
-            when /^timestamp with(?:out)? time zone$/
-              :datetime
-            else
-              :timestamp
-            end
+            :datetime
           end
 
           def type_cast(value)
diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb
index 21beed332f..9c666dcd3b 100644
--- a/activerecord/lib/active_record/relation/delegation.rb
+++ b/activerecord/lib/active_record/relation/delegation.rb
@@ -40,7 +40,7 @@ module ActiveRecord
     BLACKLISTED_ARRAY_METHODS = [
       :compact!, :flatten!, :reject!, :reverse!, :rotate!, :map!,
       :shuffle!, :slice!, :sort!, :sort_by!, :delete_if,
-      :keep_if, :pop, :shift, :delete_at, :compact
+      :keep_if, :pop, :shift, :delete_at, :compact, :select!
     ].to_set # :nodoc:
 
     delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, to: :to_a
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index 6f57441a66..db32ae12a8 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -242,7 +242,7 @@ module ActiveRecord
     # If no order is defined it will order by primary key.
     #
     #   Person.forty_two # returns the forty-second object fetched by SELECT * FROM people
-    #   Person.offset(3).forty_two # returns the fifth object from OFFSET 3 (which is OFFSET 44)
+    #   Person.offset(3).forty_two # returns the forty-second object from OFFSET 3 (which is OFFSET 44)
     #   Person.where(["user_name = :u", { u: user_name }]).forty_two
     def forty_two
       find_nth(41, offset_index)
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb
index be44fccad5..fcb28a18f6 100644
--- a/activerecord/lib/active_record/relation/merger.rb
+++ b/activerecord/lib/active_record/relation/merger.rb
@@ -30,6 +30,8 @@ module ActiveRecord
             else
               other.joins!(*v)
             end
+          elsif k == :select
+            other._select!(v)
           else
             other.send("#{k}!", v)
           end
@@ -62,7 +64,13 @@ module ActiveRecord
           # expensive), most of the time the value is going to be `nil` or `.blank?`, the only catch is that
           # `false.blank?` returns `true`, so there needs to be an extra check so that explicit `false` values
           # don't fall through the cracks.
-          relation.send("#{name}!", *value) unless value.nil? || (value.blank? && false != value)
+          unless value.nil? || (value.blank? && false != value)
+            if name == :select
+              relation._select!(*value)
+            else
+              relation.send("#{name}!", *value)
+            end
+          end
         end
 
         merge_multi_values
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 8cd97a3715..416f2305d2 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -64,6 +64,7 @@ module ActiveRecord
                                              #
         def #{name}_values=(values)          # def select_values=(values)
           raise ImmutableRelation if @loaded #   raise ImmutableRelation if @loaded
+          check_cached_relation
           @values[:#{name}] = values         #   @values[:select] = values
         end                                  # end
       CODE
@@ -81,11 +82,22 @@ module ActiveRecord
       class_eval <<-CODE, __FILE__, __LINE__ + 1
         def #{name}_value=(value)            # def readonly_value=(value)
           raise ImmutableRelation if @loaded #   raise ImmutableRelation if @loaded
+          check_cached_relation
           @values[:#{name}] = value          #   @values[:readonly] = value
         end                                  # end
       CODE
     end
 
+    def check_cached_relation # :nodoc:
+      if defined?(@arel) && @arel
+        @arel = nil
+        ActiveSupport::Deprecation.warn <<-WARNING
+Modifying already cached Relation. The cache will be reset.
+Use a cloned Relation to prevent this warning.
+WARNING
+      end
+    end
+
     def create_with_value # :nodoc:
       @values[:create_with] || {}
     end
@@ -235,11 +247,11 @@ module ActiveRecord
         to_a.select { |*block_args| yield(*block_args) }
       else
         raise ArgumentError, 'Call this with at least one field' if fields.empty?
-        spawn.select!(*fields)
+        spawn._select!(*fields)
       end
     end
 
-    def select!(*fields) # :nodoc:
+    def _select!(*fields) # :nodoc:
       fields.flatten!
       fields.map! do |field|
         klass.attribute_alias?(field) ? klass.attribute_alias(field) : field