Use SET CONSTRAINTS for disable_referential_integrity without sup…

…eruser privileges (take 2)

Re-create #21233

eeac615 was reverted (127509c) because it breaks tests.

----------------

ref: 72c1557

- We must use `authors` fixture with `author_addresses` because of its foreign key constraint.
- Tests require PostgreSQL >= 9.4.2 because it had a bug about `ALTER CONSTRAINTS` and fixed in 9.4.2.

.travis.yml

@@ -15,6 +15,9 @@ services:
 
 addons:
   postgresql: "9.4"
+  apt:
+    packages:
+      - postgresql-9.4
 
 bundler_args: --without test --jobs 3 --retry 3
 before_install:
@@ -101,6 +104,12 @@ matrix:
       jdk: oraclejdk8
       env:
         - "GEM=am,amo,aj"
+    # Test with old (< 9.4.2) postgresql
+    - rvm: 2.4.0
+      env:
+        - "GEM=ar:postgresql"
+      addons:
+        postgresql: "9.4"
   allow_failures:
     - rvm: ruby-head
     - rvm: jruby-9.1.8.0

activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb

@@ -6,8 +6,50 @@ def supports_disable_referential_integrity? # :nodoc:
           true
         end
 
-        def disable_referential_integrity # :nodoc:
+        def disable_referential_integrity(&block) # :nodoc:
           if supports_disable_referential_integrity?
+            if supports_alter_constraint?
+              disable_referential_integrity_with_alter_constraint(&block)
+            else
+              disable_referential_integrity_with_disable_trigger(&block)
+            end
+          else
+            yield
+          end
+        end
+
+        private
+
+          def disable_referential_integrity_with_alter_constraint
+            tables_constraints = execute(<<-SQL).values
+              SELECT table_name, constraint_name
+              FROM information_schema.table_constraints
+              WHERE constraint_type = 'FOREIGN KEY'
+              AND is_deferrable = 'NO'
+            SQL
+
+            execute(
+              tables_constraints.collect { |table, constraint|
+                "ALTER TABLE #{quote_table_name(table)} ALTER CONSTRAINT #{constraint} DEFERRABLE"
+              }.join(";")
+            )
+
+            begin
+              transaction do
+                execute("SET CONSTRAINTS ALL DEFERRED")
+
+                yield
+              end
+            ensure
+              execute(
+                tables_constraints.collect { |table, constraint|
+                  "ALTER TABLE #{quote_table_name(table)} ALTER CONSTRAINT #{constraint} NOT DEFERRABLE"
+                }.join(";")
+              )
+            end
+          end
+
+          def disable_referential_integrity_with_disable_trigger
             original_exception = nil
 
             begin
@@ -39,10 +81,7 @@ def disable_referential_integrity # :nodoc:
               end
             rescue ActiveRecord::ActiveRecordError
             end
-          else
-            yield
           end
-        end
       end
     end
   end

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -314,6 +314,12 @@ def supports_pgcrypto_uuid?
         postgresql_version >= 90400
       end
 
+      def supports_alter_constraint?
+        # PostgreSQL 9.4 introduces ALTER TABLE ... ALTER CONSTRAINT but it has a bug and fixed in 9.4.2
+        # https://www.postgresql.org/docs/9.4/static/release-9-4-2.html
+        postgresql_version >= 90402
+      end
+
       def get_advisory_lock(lock_id) # :nodoc:
         unless lock_id.is_a?(Integer) && lock_id.bit_length <= 63
           raise(ArgumentError, "Postgres requires advisory lock ids to be a signed 64 bit integer")

activerecord/test/cases/adapters/postgresql/referential_integrity_test.rb

@@ -1,111 +1,150 @@
 require "cases/helper"
 require "support/connection_helper"
 
-class PostgreSQLReferentialIntegrityTest < ActiveRecord::PostgreSQLTestCase
-  self.use_transactional_tests = false
+if ActiveRecord::Base.connection.respond_to?(:supports_alter_constraint?) &&
+    ActiveRecord::Base.connection.supports_alter_constraint?
+  class PostgreSQLReferentialIntegrityWithAlterConstraintTest < ActiveRecord::PostgreSQLTestCase
+    self.use_transactional_tests = false
 
-  include ConnectionHelper
+    include ConnectionHelper
 
-  IS_REFERENTIAL_INTEGRITY_SQL = lambda do |sql|
-    sql.match(/DISABLE TRIGGER ALL/) || sql.match(/ENABLE TRIGGER ALL/)
-  end
+    IS_REFERENTIAL_INTEGRITY_SQL = lambda do |sql|
+      sql.match(/SET CONSTRAINTS ALL DEFERRED/)
+    end
 
-  module MissingSuperuserPrivileges
-    def execute(sql)
-      if IS_REFERENTIAL_INTEGRITY_SQL.call(sql)
-        super "BROKEN;" rescue nil # put transaction in broken state
-        raise ActiveRecord::StatementInvalid, "PG::InsufficientPrivilege"
-      else
-        super
+    module ProgrammerMistake
+      def execute(sql)
+        if IS_REFERENTIAL_INTEGRITY_SQL.call(sql)
+          raise ArgumentError, "something is not right."
+        else
+          super
+        end
       end
     end
-  end
 
-  module ProgrammerMistake
-    def execute(sql)
-      if IS_REFERENTIAL_INTEGRITY_SQL.call(sql)
-        raise ArgumentError, "something is not right."
-      else
-        super
+    def setup
+      @connection = ActiveRecord::Base.connection
+    end
+
+    def teardown
+      reset_connection
+    end
+
+    def test_errors_bubble_up
+      @connection.extend ProgrammerMistake
+
+      assert_raises ArgumentError do
+        @connection.disable_referential_integrity {}
       end
     end
   end
+else
+  class PostgreSQLReferentialIntegrityWithDisableTriggerTest < ActiveRecord::PostgreSQLTestCase
+    self.use_transactional_tests = false
 
-  def setup
-    @connection = ActiveRecord::Base.connection
-  end
+    include ConnectionHelper
 
-  def teardown
-    reset_connection
-    if ActiveRecord::Base.connection.is_a?(MissingSuperuserPrivileges)
-      raise "MissingSuperuserPrivileges patch was not removed"
+    IS_REFERENTIAL_INTEGRITY_SQL = lambda do |sql|
+      sql.match(/DISABLE TRIGGER ALL/) || sql.match(/ENABLE TRIGGER ALL/)
     end
-  end
 
-  def test_should_reraise_invalid_foreign_key_exception_and_show_warning
-    @connection.extend MissingSuperuserPrivileges
+    module MissingSuperuserPrivileges
+      def execute(sql)
+        if IS_REFERENTIAL_INTEGRITY_SQL.call(sql)
+          super "BROKEN;" rescue nil # put transaction in broken state
+          raise ActiveRecord::StatementInvalid, "PG::InsufficientPrivilege"
+        else
+          super
+        end
+      end
+    end
 
-    warning = capture(:stderr) do
-      e = assert_raises(ActiveRecord::InvalidForeignKey) do
-        @connection.disable_referential_integrity do
-          raise ActiveRecord::InvalidForeignKey, "Should be re-raised"
+    module ProgrammerMistake
+      def execute(sql)
+        if IS_REFERENTIAL_INTEGRITY_SQL.call(sql)
+          raise ArgumentError, "something is not right."
+        else
+          super
         end
       end
-      assert_equal "Should be re-raised", e.message
     end
-    assert_match (/WARNING: Rails was not able to disable referential integrity/), warning
-    assert_match (/cause: PG::InsufficientPrivilege/), warning
-  end
 
-  def test_does_not_print_warning_if_no_invalid_foreign_key_exception_was_raised
-    @connection.extend MissingSuperuserPrivileges
+    def setup
+      @connection = ActiveRecord::Base.connection
+    end
 
-    warning = capture(:stderr) do
-      e = assert_raises(ActiveRecord::StatementInvalid) do
-        @connection.disable_referential_integrity do
-          raise ActiveRecord::StatementInvalid, "Should be re-raised"
+    def teardown
+      reset_connection
+      if ActiveRecord::Base.connection.is_a?(MissingSuperuserPrivileges)
+        raise "MissingSuperuserPrivileges patch was not removed"
+      end
+    end
+
+    def test_should_reraise_invalid_foreign_key_exception_and_show_warning
+      @connection.extend MissingSuperuserPrivileges
+
+      warning = capture(:stderr) do
+        e = assert_raises(ActiveRecord::InvalidForeignKey) do
+          @connection.disable_referential_integrity do
+            raise ActiveRecord::InvalidForeignKey, "Should be re-raised"
+          end
         end
+        assert_equal "Should be re-raised", e.message
       end
-      assert_equal "Should be re-raised", e.message
+      assert_match (/WARNING: Rails was not able to disable referential integrity/), warning
+      assert_match (/cause: PG::InsufficientPrivilege/), warning
     end
-    assert warning.blank?, "expected no warnings but got:\n#{warning}"
-  end
 
-  def test_does_not_break_transactions
-    @connection.extend MissingSuperuserPrivileges
+    def test_does_not_print_warning_if_no_invalid_foreign_key_exception_was_raised
+      @connection.extend MissingSuperuserPrivileges
 
-    @connection.transaction do
-      @connection.disable_referential_integrity do
-        assert_transaction_is_not_broken
+      warning = capture(:stderr) do
+        e = assert_raises(ActiveRecord::StatementInvalid) do
+          @connection.disable_referential_integrity do
+            raise ActiveRecord::StatementInvalid, "Should be re-raised"
+          end
+        end
+        assert_equal "Should be re-raised", e.message
       end
-      assert_transaction_is_not_broken
+      assert warning.blank?, "expected no warnings but got:\n#{warning}"
     end
-  end
 
-  def test_does_not_break_nested_transactions
-    @connection.extend MissingSuperuserPrivileges
+    def test_does_not_break_transactions
+      @connection.extend MissingSuperuserPrivileges
 
-    @connection.transaction do
-      @connection.transaction(requires_new: true) do
+      @connection.transaction do
         @connection.disable_referential_integrity do
           assert_transaction_is_not_broken
         end
+        assert_transaction_is_not_broken
       end
-      assert_transaction_is_not_broken
     end
-  end
 
-  def test_only_catch_active_record_errors_others_bubble_up
-    @connection.extend ProgrammerMistake
+    def test_does_not_break_nested_transactions
+      @connection.extend MissingSuperuserPrivileges
 
-    assert_raises ArgumentError do
-      @connection.disable_referential_integrity {}
+      @connection.transaction do
+        @connection.transaction(requires_new: true) do
+          @connection.disable_referential_integrity do
+            assert_transaction_is_not_broken
+          end
+        end
+        assert_transaction_is_not_broken
+      end
     end
-  end
 
-  private
+    def test_only_catch_active_record_errors_others_bubble_up
+      @connection.extend ProgrammerMistake
 
-    def assert_transaction_is_not_broken
-      assert_equal 1, @connection.select_value("SELECT 1")
+      assert_raises ArgumentError do
+        @connection.disable_referential_integrity {}
+      end
     end
+
+    private
+
+      def assert_transaction_is_not_broken
+        assert_equal 1, @connection.select_value("SELECT 1")
+      end
+  end
 end

activerecord/test/cases/associations/callbacks_test.rb

@@ -7,7 +7,7 @@
 require "models/company"
 
 class AssociationCallbacksTest < ActiveRecord::TestCase
-  fixtures :posts, :authors, :projects, :developers
+  fixtures :posts, :authors, :author_addresses, :projects, :developers
 
   def setup
     @david = authors(:david)

activerecord/test/cases/associations/cascaded_eager_loading_test.rb

@@ -12,7 +12,7 @@
 require "models/edge"
 
 class CascadedEagerLoadingTest < ActiveRecord::TestCase
-  fixtures :authors, :mixins, :companies, :posts, :topics, :accounts, :comments,
+  fixtures :authors, :author_addresses, :mixins, :companies, :posts, :topics, :accounts, :comments,
            :categorizations, :people, :categories, :edges, :vertices
 
   def test_eager_association_loading_with_cascaded_two_levels

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -40,7 +40,7 @@
 require "models/interest"
 
 class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase
-  fixtures :authors, :posts, :comments
+  fixtures :authors, :author_addresses, :posts, :comments
 
   def test_should_generate_valid_sql
     author = authors(:david)
@@ -51,7 +51,7 @@ def test_should_generate_valid_sql
 end
 
 class HasManyAssociationsTestPrimaryKeys < ActiveRecord::TestCase
-  fixtures :authors, :essays, :subscribers, :subscriptions, :people
+  fixtures :authors, :author_addresses, :essays, :subscribers, :subscriptions, :people
 
   def test_custom_primary_key_on_new_record_should_fetch_with_query
     subscriber = Subscriber.new(nick: "webster132")
@@ -100,7 +100,7 @@ def test_blank_custom_primary_key_on_new_record_should_not_run_queries
 
 class HasManyAssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :categories, :companies, :developers, :projects,
-           :developers_projects, :topics, :authors, :comments,
+           :developers_projects, :topics, :authors, :author_addresses, :comments,
            :posts, :readers, :taggings, :cars, :jobs, :tags,
            :categorizations, :zines, :interests
 

activerecord/test/cases/associations/has_one_through_associations_test.rb

@@ -23,7 +23,7 @@
 
 class HasOneThroughAssociationsTest < ActiveRecord::TestCase
   fixtures :member_types, :members, :clubs, :memberships, :sponsors, :organizations, :minivans,
-           :dashboards, :speedometers, :authors, :posts, :comments, :categories, :essays, :owners
+           :dashboards, :speedometers, :authors, :author_addresses, :posts, :comments, :categories, :essays, :owners
 
   def setup
     @member = members(:groucho)

activerecord/test/cases/associations/inner_join_association_test.rb

@@ -10,7 +10,7 @@
 require "models/tag"
 
 class InnerJoinAssociationTest < ActiveRecord::TestCase
-  fixtures :authors, :essays, :posts, :comments, :categories, :categories_posts, :categorizations,
+  fixtures :authors, :author_addresses, :essays, :posts, :comments, :categories, :categories_posts, :categorizations,
            :taggings, :tags
 
   def test_construct_finder_sql_applies_aliases_tables_on_association_conditions

activerecord/test/cases/associations/join_model_test.rb

@@ -19,7 +19,7 @@
 class AssociationsJoinModelTest < ActiveRecord::TestCase
   self.use_transactional_tests = false unless supports_savepoints?
 
-  fixtures :posts, :authors, :categories, :categorizations, :comments, :tags, :taggings, :author_favorites, :vertices, :items, :books,
+  fixtures :posts, :authors, :author_addresses, :categories, :categorizations, :comments, :tags, :taggings, :author_favorites, :vertices, :items, :books,
     # Reload edges table from fixtures as otherwise repeated test was failing
     :edges
 

activerecord/test/cases/associations/nested_through_associations_test.rb

@@ -24,7 +24,7 @@
 require "models/essay"
 
 class NestedThroughAssociationsTest < ActiveRecord::TestCase
-  fixtures :authors, :books, :posts, :subscriptions, :subscribers, :tags, :taggings,
+  fixtures :authors, :author_addresses, :books, :posts, :subscriptions, :subscribers, :tags, :taggings,
            :people, :readers, :references, :jobs, :ratings, :comments, :members, :member_details,
            :member_types, :sponsors, :clubs, :organizations, :categories, :categories_posts,
            :categorizations, :memberships, :essays