Merge branch 'master' into custom-discarded-job-handling

5 files changed, 202 insertions, 90 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 0759f4d2b3..9849f9d5d7 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -63,15 +63,13 @@ module ActiveRecord
     # There are several connection-pooling-related options that you can add to
     # your database connection configuration:
     #
-    # * +pool+: number indicating size of connection pool (default 5)
-    # * +checkout_timeout+: number of seconds to block and wait for a connection
-    #   before giving up and raising a timeout error (default 5 seconds).
-    # * +reaping_frequency+: frequency in seconds to periodically run the
-    #   Reaper, which attempts to find and recover connections from dead
-    #   threads, which can occur if a programmer forgets to close a
-    #   connection at the end of a thread or a thread dies unexpectedly.
-    #   Regardless of this setting, the Reaper will be invoked before every
-    #   blocking wait. (Default +nil+, which means don't schedule the Reaper).
+    # * +pool+: maximum number of connections the pool may manage (default 5).
+    # * +idle_timeout+: number of seconds that a connection will be kept
+    #   unused in the pool before it is automatically disconnected (default
+    #   300 seconds). Set this to zero to keep connections forever.
+    # * +checkout_timeout+: number of seconds to wait for a connection to
+    #   become available before giving up and raising a timeout error (default
+    #   5 seconds).
     #
     #--
     # Synchronization policy:
@@ -82,11 +80,8 @@ module ActiveRecord
     # * private methods that require being called in a +synchronize+ blocks
     #   are now explicitly documented
     class ConnectionPool
-      # Threadsafe, fair, FIFO queue.  Meant to be used by ConnectionPool
-      # with which it shares a Monitor.  But could be a generic Queue.
-      #
-      # The Queue in stdlib's 'thread' could replace this class except
-      # stdlib's doesn't support waiting with a timeout.
+      # Threadsafe, fair, LIFO queue.  Meant to be used by ConnectionPool
+      # with which it shares a Monitor.
       class Queue
         def initialize(lock = Monitor.new)
           @lock = lock
@@ -175,7 +170,7 @@ module ActiveRecord
 
           # Removes and returns the head of the queue if possible, or +nil+.
           def remove
-            @queue.shift
+            @queue.pop
           end
 
           # Remove and return the head the queue if the number of
@@ -283,12 +278,12 @@ module ActiveRecord
           end
       end
 
-      # Every +frequency+ seconds, the reaper will call +reap+ on +pool+.
-      # A reaper instantiated with a +nil+ frequency will never reap the
-      # connection pool.
+      # Every +frequency+ seconds, the reaper will call +reap+ and +flush+ on
+      # +pool+. A reaper instantiated with a zero frequency will never reap
+      # the connection pool.
       #
-      # Configure the frequency by setting "reaping_frequency" in your
-      # database yaml file.
+      # Configure the frequency by setting +reaping_frequency+ in your database
+      # yaml file (default 60 seconds).
       class Reaper
         attr_reader :pool, :frequency
 
@@ -298,11 +293,12 @@ module ActiveRecord
         end
 
         def run
-          return unless frequency
+          return unless frequency && frequency > 0
           Thread.new(frequency, pool) { |t, p|
             loop do
               sleep t
               p.reap
+              p.flush
             end
           }
         end
@@ -326,6 +322,10 @@ module ActiveRecord
         @spec = spec
 
         @checkout_timeout = (spec.config[:checkout_timeout] && spec.config[:checkout_timeout].to_f) || 5
+        if @idle_timeout = spec.config.fetch(:idle_timeout, 300)
+          @idle_timeout = @idle_timeout.to_f
+          @idle_timeout = nil if @idle_timeout <= 0
+        end
 
         # default max pool size to 5
         @size = (spec.config[:pool] && spec.config[:pool].to_i) || 5
@@ -356,7 +356,10 @@ module ActiveRecord
 
         @lock_thread = false
 
-        @reaper = Reaper.new(self, spec.config[:reaping_frequency] && spec.config[:reaping_frequency].to_f)
+        # +reaping_frequency+ is configurable mostly for historical reasons, but it could
+        # also be useful if someone wants a very low +idle_timeout+.
+        reaping_frequency = spec.config.fetch(:reaping_frequency, 60)
+        @reaper = Reaper.new(self, reaping_frequency && reaping_frequency.to_f)
         @reaper.run
       end
 
@@ -450,6 +453,21 @@ module ActiveRecord
         disconnect(false)
       end
 
+      # Discards all connections in the pool (even if they're currently
+      # leased!), along with the pool itself. Any further interaction with the
+      # pool (except #spec and #schema_cache) is undefined.
+      #
+      # See AbstractAdapter#discard!
+      def discard! # :nodoc:
+        synchronize do
+          return if @connections.nil? # already discarded
+          @connections.each do |conn|
+            conn.discard!
+          end
+          @connections = @available = @thread_cached_conns = nil
+        end
+      end
+
       # Clears the cache which maps classes and re-connects connections that
       # require reloading.
       #
@@ -575,6 +593,35 @@ module ActiveRecord
         end
       end
 
+      # Disconnect all connections that have been idle for at least
+      # +minimum_idle+ seconds. Connections currently checked out, or that were
+      # checked in less than +minimum_idle+ seconds ago, are unaffected.
+      def flush(minimum_idle = @idle_timeout)
+        return if minimum_idle.nil?
+
+        idle_connections = synchronize do
+          @connections.select do |conn|
+            !conn.in_use? && conn.seconds_idle >= minimum_idle
+          end.each do |conn|
+            conn.lease
+
+            @available.delete conn
+            @connections.delete conn
+          end
+        end
+
+        idle_connections.each do |conn|
+          conn.disconnect!
+        end
+      end
+
+      # Disconnect all currently idle connections. Connections currently checked
+      # out are unaffected.
+      def flush!
+        reap
+        flush(-1)
+      end
+
       def num_waiting_in_queue # :nodoc:
         @available.num_waiting
       end
@@ -866,11 +913,31 @@ module ActiveRecord
     # about the model. The model needs to pass a specification name to the handler,
     # in order to look up the correct connection pool.
     class ConnectionHandler
+      def self.unowned_pool_finalizer(pid_map) # :nodoc:
+        lambda do |_|
+          discard_unowned_pools(pid_map)
+        end
+      end
+
+      def self.discard_unowned_pools(pid_map) # :nodoc:
+        pid_map.each do |pid, pools|
+          pools.values.compact.each(&:discard!) unless pid == Process.pid
+        end
+      end
+
       def initialize
         # These caches are keyed by spec.name (ConnectionSpecification#name).
         @owner_to_pool = Concurrent::Map.new(initial_capacity: 2) do |h, k|
+          # Discard the parent's connection pools immediately; we have no need
+          # of them
+          ConnectionHandler.discard_unowned_pools(h)
+
           h[k] = Concurrent::Map.new(initial_capacity: 2)
         end
+
+        # Backup finalizer: if the forked child never needed a pool, the above
+        # early discard has not occurred
+        ObjectSpace.define_finalizer self, ConnectionHandler.unowned_pool_finalizer(@owner_to_pool)
       end
 
       def connection_pool_list
@@ -924,6 +991,13 @@ module ActiveRecord
         connection_pool_list.each(&:disconnect!)
       end
 
+      # Disconnects all currently idle connections.
+      #
+      # See ConnectionPool#flush! for details.
+      def flush_idle_connections!
+        connection_pool_list.each(&:flush!)
+      end
+
       # Locate the connection of the nearest super class. This can be an
       # active or defined connection: if it is the latter, it will be
       # opened and set as the active connection for the class it was defined
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
index 9ad04c3216..92e46ccf9f 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
@@ -11,19 +11,6 @@ module ActiveRecord
       def quote(value)
         value = id_value_for_database(value) if value.is_a?(Base)
 
-        if value.respond_to?(:quoted_id)
-          at = value.method(:quoted_id).source_location
-          at &&= " at %s:%d" % at
-
-          owner = value.method(:quoted_id).owner.to_s
-          klass = value.class.to_s
-          klass += "(#{owner})" unless owner == klass
-
-          ActiveSupport::Deprecation.warn \
-            "Defining #quoted_id is deprecated and will be ignored in Rails 5.2. (defined on #{klass}#{at})"
-          return value.quoted_id
-        end
-
         if value.respond_to?(:value_for_database)
           value = value.value_for_database
         end
@@ -37,10 +24,6 @@ module ActiveRecord
       def type_cast(value, column = nil)
         value = id_value_for_database(value) if value.is_a?(Base)
 
-        if value.respond_to?(:quoted_id) && value.respond_to?(:id)
-          return value.id
-        end
-
         if column
           value = type_cast_from_column(column, value)
         end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index 788a455773..0594b4b485 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -6,7 +6,7 @@ module ActiveRecord
     # this type are typically created and returned by methods in database
     # adapters. e.g. ActiveRecord::ConnectionAdapters::MySQL::SchemaStatements#indexes
     class IndexDefinition # :nodoc:
-      attr_reader :table, :name, :unique, :columns, :lengths, :orders, :where, :type, :using, :comment
+      attr_reader :table, :name, :unique, :columns, :lengths, :orders, :opclasses, :where, :type, :using, :comment
 
       def initialize(
         table, name,
@@ -14,6 +14,7 @@ module ActiveRecord
         columns = [],
         lengths: {},
         orders: {},
+        opclasses: {},
         where: nil,
         type: nil,
         using: nil,
@@ -23,13 +24,23 @@ module ActiveRecord
         @name = name
         @unique = unique
         @columns = columns
-        @lengths = lengths
-        @orders = orders
+        @lengths = concise_options(lengths)
+        @orders = concise_options(orders)
+        @opclasses = concise_options(opclasses)
         @where = where
         @type = type
         @using = using
         @comment = comment
       end
+
+      private
+        def concise_options(options)
+          if columns.size == options.size && options.values.uniq.size == 1
+            options.values.first
+          else
+            options
+          end
+        end
     end
 
     # Abstract representation of a column definition. Instances of this type
@@ -85,6 +96,11 @@ module ActiveRecord
         options[:primary_key] != default_primary_key
       end
 
+      def validate?
+        options.fetch(:validate, true)
+      end
+      alias validated? validate?
+
       def defined_for?(to_table_ord = nil, to_table: nil, **options)
         if to_table_ord
           self.to_table == to_table_ord.to_s
@@ -204,6 +220,7 @@ module ActiveRecord
         :decimal,
         :float,
         :integer,
+        :json,
         :string,
         :text,
         :time,
@@ -396,6 +413,9 @@ module ActiveRecord
       alias :belongs_to :references
 
       def new_column_definition(name, type, **options) # :nodoc:
+        if integer_like_primary_key?(type, options)
+          type = integer_like_primary_key_type(type, options)
+        end
         type = aliased_types(type.to_s, type)
         options[:primary_key] ||= type == :primary_key
         options[:null] = false if options[:primary_key]
@@ -410,6 +430,14 @@ module ActiveRecord
         def aliased_types(name, fallback)
           "timestamp" == name ? :datetime : fallback
         end
+
+        def integer_like_primary_key?(type, options)
+          options[:primary_key] && [:integer, :bigint].include?(type) && !options.key?(:default)
+        end
+
+        def integer_like_primary_key_type(type, options)
+          type
+        end
     end
 
     class AlterTable # :nodoc:
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index f57c7a5d4d..4f58b0242c 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
-require_relative "../../migration/join_table"
+require "active_record/migration/join_table"
 require "active_support/core_ext/string/access"
-require "digest"
+require "digest/sha2"
 
 module ActiveRecord
   module ConnectionAdapters # :nodoc:
@@ -79,7 +79,7 @@ module ActiveRecord
       end
 
       # Returns an array of indexes for the given table.
-      def indexes(table_name, name = nil)
+      def indexes(table_name)
         raise NotImplementedError, "#indexes is not implemented"
       end
 
@@ -216,7 +216,7 @@ module ActiveRecord
       # generates:
       #
       #   CREATE TABLE suppliers (
-      #     id int auto_increment PRIMARY KEY
+      #     id bigint auto_increment PRIMARY KEY
       #   ) ENGINE=InnoDB DEFAULT CHARSET=utf8
       #
       # ====== Rename the primary key column
@@ -228,7 +228,7 @@ module ActiveRecord
       # generates:
       #
       #   CREATE TABLE objects (
-      #     guid int auto_increment PRIMARY KEY,
+      #     guid bigint auto_increment PRIMARY KEY,
       #     name varchar(80)
       #   )
       #
@@ -255,8 +255,8 @@ module ActiveRecord
       # generates:
       #
       #   CREATE TABLE order (
-      #       product_id integer NOT NULL,
-      #       client_id integer NOT NULL
+      #       product_id bigint NOT NULL,
+      #       client_id bigint NOT NULL
       #   );
       #
       #   ALTER TABLE ONLY "orders"
@@ -265,15 +265,15 @@ module ActiveRecord
       # ====== Do not add a primary key column
       #
       #   create_table(:categories_suppliers, id: false) do |t|
-      #     t.column :category_id, :integer
-      #     t.column :supplier_id, :integer
+      #     t.column :category_id, :bigint
+      #     t.column :supplier_id, :bigint
       #   end
       #
       # generates:
       #
       #   CREATE TABLE categories_suppliers (
-      #     category_id int,
-      #     supplier_id int
+      #     category_id bigint,
+      #     supplier_id bigint
       #   )
       #
       # ====== Create a temporary table based on a query
@@ -361,8 +361,8 @@ module ActiveRecord
       # generates:
       #
       #   CREATE TABLE assemblies_parts (
-      #     assembly_id int NOT NULL,
-      #     part_id int NOT NULL,
+      #     assembly_id bigint NOT NULL,
+      #     part_id bigint NOT NULL,
       #   ) ENGINE=InnoDB DEFAULT CHARSET=utf8
       #
       def create_join_table(table_1, table_2, column_options: {}, **options)
@@ -432,7 +432,7 @@ module ActiveRecord
       #     t.references :company
       #   end
       #
-      # Creates a <tt>company_id(integer)</tt> column.
+      # Creates a <tt>company_id(bigint)</tt> column.
       #
       # ====== Add a polymorphic foreign key column
       #
@@ -440,7 +440,7 @@ module ActiveRecord
       #    t.belongs_to :company, polymorphic: true
       #  end
       #
-      # Creates <tt>company_type(varchar)</tt> and <tt>company_id(integer)</tt> columns.
+      # Creates <tt>company_type(varchar)</tt> and <tt>company_id(bigint)</tt> columns.
       #
       # ====== Remove a column
       #
@@ -522,6 +522,8 @@ module ActiveRecord
       #   Specifies the precision for the <tt>:decimal</tt> and <tt>:numeric</tt> columns.
       # * <tt>:scale</tt> -
       #   Specifies the scale for the <tt>:decimal</tt> and <tt>:numeric</tt> columns.
+      # * <tt>:comment</tt> -
+      #   Specifies the comment for the column. This option is ignored by some backends.
       #
       # Note: The precision is the total number of significant digits,
       # and the scale is the number of digits that can be stored following
@@ -598,7 +600,7 @@ module ActiveRecord
       # to provide these in a migration's +change+ method so it can be reverted.
       # In that case, +type+ and +options+ will be used by #add_column.
       def remove_column(table_name, column_name, type = nil, options = {})
-        execute "ALTER TABLE #{quote_table_name(table_name)} DROP #{quote_column_name(column_name)}"
+        execute "ALTER TABLE #{quote_table_name(table_name)} #{remove_column_for_alter(table_name, column_name, type, options)}"
       end
 
       # Changes the column's definition according to the new options.
@@ -736,6 +738,28 @@ module ActiveRecord
       #
       # Note: only supported by PostgreSQL and MySQL
       #
+      # ====== Creating an index with a specific operator class
+      #
+      #   add_index(:developers, :name, using: 'gist', opclass: :gist_trgm_ops)
+      #
+      # generates:
+      #
+      #   CREATE INDEX developers_on_name ON developers USING gist (name gist_trgm_ops) -- PostgreSQL
+      #
+      #   add_index(:developers, [:name, :city], using: 'gist', opclass: { city: :gist_trgm_ops })
+      #
+      # generates:
+      #
+      #   CREATE INDEX developers_on_name_and_city ON developers USING gist (name, city gist_trgm_ops) -- PostgreSQL
+      #
+      #   add_index(:developers, [:name, :city], using: 'gist', opclass: :gist_trgm_ops)
+      #
+      # generates:
+      #
+      #   CREATE INDEX developers_on_name_and_city ON developers USING gist (name gist_trgm_ops, city gist_trgm_ops) -- PostgreSQL
+      #
+      # Note: only supported by PostgreSQL
+      #
       # ====== Creating an index with a specific type
       #
       #   add_index(:developers, :name, type: :fulltext)
@@ -804,24 +828,19 @@ module ActiveRecord
       end
 
       # Verifies the existence of an index with a given name.
-      def index_name_exists?(table_name, index_name, default = nil)
-        unless default.nil?
-          ActiveSupport::Deprecation.warn(<<-MSG.squish)
-            Passing default to #index_name_exists? is deprecated without replacement.
-          MSG
-        end
+      def index_name_exists?(table_name, index_name)
         index_name = index_name.to_s
         indexes(table_name).detect { |i| i.name == index_name }
       end
 
-      # Adds a reference. The reference column is an integer by default,
+      # Adds a reference. The reference column is a bigint by default,
       # the <tt>:type</tt> option can be used to specify a different type.
       # Optionally adds a +_type+ column, if <tt>:polymorphic</tt> option is provided.
       # #add_reference and #add_belongs_to are acceptable.
       #
       # The +options+ hash can include the following keys:
       # [<tt>:type</tt>]
-      #   The reference column type. Defaults to +:integer+.
+      #   The reference column type. Defaults to +:bigint+.
       # [<tt>:index</tt>]
       #   Add an appropriate index. Defaults to true.
       #   See #add_index for usage of this option.
@@ -832,7 +851,7 @@ module ActiveRecord
       # [<tt>:null</tt>]
       #   Whether the column allows nulls. Defaults to true.
       #
-      # ====== Create a user_id integer column
+      # ====== Create a user_id bigint column
       #
       #   add_reference(:products, :user)
       #
@@ -945,6 +964,8 @@ module ActiveRecord
       #   Action that happens <tt>ON DELETE</tt>. Valid values are +:nullify+, +:cascade+ and +:restrict+
       # [<tt>:on_update</tt>]
       #   Action that happens <tt>ON UPDATE</tt>. Valid values are +:nullify+, +:cascade+ and +:restrict+
+      # [<tt>:validate</tt>]
+      #   (Postgres only) Specify whether or not the constraint should be validated. Defaults to +true+.
       def add_foreign_key(from_table, to_table, options = {})
         return unless supports_foreign_keys?
 
@@ -1018,16 +1039,6 @@ module ActiveRecord
         insert_versions_sql(versions) if versions.any?
       end
 
-      def initialize_schema_migrations_table # :nodoc:
-        ActiveRecord::SchemaMigration.create_table
-      end
-      deprecate :initialize_schema_migrations_table
-
-      def initialize_internal_metadata_table # :nodoc:
-        ActiveRecord::InternalMetadata.create_table
-      end
-      deprecate :initialize_internal_metadata_table
-
       def internal_string_options_for_primary_key # :nodoc:
         { primary_key: true }
       end
@@ -1133,7 +1144,7 @@ module ActiveRecord
       def add_index_options(table_name, column_name, comment: nil, **options) # :nodoc:
         column_names = index_column_names(column_name)
 
-        options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type)
+        options.assert_valid_keys(:unique, :order, :name, :where, :length, :internal, :using, :algorithm, :type, :opclass)
 
         index_type = options[:type].to_s if options.key?(:type)
         index_type ||= options[:unique] ? "UNIQUE" : ""
@@ -1172,7 +1183,7 @@ module ActiveRecord
       end
 
       # Changes the comment for a column or removes it if +nil+.
-      def change_column_comment(table_name, column_name, comment) #:nodoc:
+      def change_column_comment(table_name, column_name, comment)
         raise NotImplementedError, "#{self.class} does not support changing column comments"
       end
 
@@ -1186,20 +1197,22 @@ module ActiveRecord
         end
 
         def add_index_sort_order(quoted_columns, **options)
-          if order = options[:order]
-            case order
-            when Hash
-              order = order.symbolize_keys
-              quoted_columns.each { |name, column| column << " #{order[name].upcase}" if order[name].present? }
-            when String
-              quoted_columns.each { |name, column| column << " #{order.upcase}" if order.present? }
-            end
+          orders = options_for_index_columns(options[:order])
+          quoted_columns.each do |name, column|
+            column << " #{orders[name].upcase}" if orders[name].present?
           end
+        end
 
-          quoted_columns
+        def options_for_index_columns(options)
+          if options.is_a?(Hash)
+            options.symbolize_keys
+          else
+            Hash.new { |hash, column| hash[column] = options }
+          end
         end
 
-        # Overridden by the MySQL adapter for supporting index lengths
+        # Overridden by the MySQL adapter for supporting index lengths and by
+        # the PostgreSQL adapter for supporting operator classes.
         def add_options_for_index_columns(quoted_columns, **options)
           if supports_index_sort_order?
             quoted_columns = add_index_sort_order(quoted_columns, options)
@@ -1353,6 +1366,20 @@ module ActiveRecord
           options.is_a?(Hash) && options.key?(:name) && options.except(:name, :algorithm).empty?
         end
 
+        def add_column_for_alter(table_name, column_name, type, options = {})
+          td = create_table_definition(table_name)
+          cd = td.new_column_definition(column_name, type, options)
+          schema_creation.accept(AddColumnDefinition.new(cd))
+        end
+
+        def remove_column_for_alter(table_name, column_name, type = nil, options = {})
+          "DROP COLUMN #{quote_column_name(column_name)}"
+        end
+
+        def remove_columns_for_alter(table_name, *column_names)
+          column_names.map { |column_name| remove_column_for_alter(table_name, column_name) }
+        end
+
         def insert_versions_sql(versions)
           sm_table = quote_table_name(ActiveRecord::SchemaMigration.table_name)
 
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
index 147e16e9fa..d9ac8db6a8 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb
@@ -240,7 +240,7 @@ module ActiveRecord
                 rollback_transaction if transaction
               else
                 begin
-                  commit_transaction
+                  commit_transaction if transaction
                 rescue Exception
                   rollback_transaction(transaction) unless transaction.state.completed?
                   raise