Merge pull request #137 from cockroachdb/feature/rails-6-update

Rails 6 update

Author: rafiss

activerecord-cockroachdb-adapter.gemspec

@@ -4,7 +4,7 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
 Gem::Specification.new do |spec|
   spec.name          = "activerecord-cockroachdb-adapter"
-  spec.version       = "5.2.1"
+  spec.version       = "6.0.0beta1"
   spec.licenses      = ["Apache-2.0"]
   spec.authors       = ["Cockroach Labs"]
   spec.email         = ["[email protected]"]
@@ -13,7 +13,7 @@ Gem::Specification.new do |spec|
   spec.description   = "Allows the use of CockroachDB as a backend for ActiveRecord and Rails apps."
   spec.homepage      = "https://github.com/cockroachdb/activerecord-cockroachdb-adapter"
 
-  spec.add_dependency "activerecord", "~> 5.2"
+  spec.add_dependency "activerecord", "~> 6.0.3"
   spec.add_dependency "pg", ">= 0.20"
 
   # Prevent pushing this gem to RubyGems.org. To allow pushes either set the 'allowed_push_host'

lib/active_record/connection_adapters/cockroachdb/schema_statements.rb

@@ -57,10 +57,35 @@ def type_to_sql(type, limit: nil, precision: nil, scale: nil, array: nil, **) #
             end
           # The call to super might have appeneded [] already.
           if array && type != :primary_key && !sql.end_with?("[]")
-            sql = "#{sql}[]" 
+            sql = "#{sql}[]"
           end
           sql
         end
+
+        # This overrides the method from PostegreSQL adapter
+        # Resets the sequence of a table's primary key to the maximum value.
+        def reset_pk_sequence!(table, pk = nil, sequence = nil)
+          unless pk && sequence
+            default_pk, default_sequence = pk_and_sequence_for(table)
+
+            pk ||= default_pk
+            sequence ||= default_sequence
+          end
+
+          if @logger && pk && !sequence
+            @logger.warn "#{table} has primary key #{pk} with no default sequence."
+          end
+
+          if pk && sequence
+            quoted_sequence = quote_table_name(sequence)
+            max_pk = query_value("SELECT MAX(#{quote_column_name pk}) FROM #{quote_table_name(table)}", "SCHEMA")
+            if max_pk.nil?
+              minvalue = query_value("SELECT seqmin FROM pg_sequence WHERE seqrelid = #{quote(quoted_sequence)}::regclass", "SCHEMA")
+            end
+
+            query_value("SELECT setval(#{quote(quoted_sequence)}, #{max_pk ? max_pk : minvalue}, #{max_pk ? true : false})", "SCHEMA")
+          end
+        end
       end
     end
   end

lib/active_record/connection_adapters/cockroachdb_adapter.rb

@@ -27,7 +27,14 @@ def cockroachdb_connection(config)
       # The postgres drivers don't allow the creation of an unconnected
       # PG::Connection object, so just pass a nil connection object for the
       # time being.
-      ConnectionAdapters::CockroachDBAdapter.new(nil, logger, conn_params, config)
+      conn = PG.connect(conn_params)
+      ConnectionAdapters::CockroachDBAdapter.new(conn, logger, conn_params, config)
+    rescue ::PG::Error => error
+      if error.message.include?("does not exist")
+        raise ActiveRecord::NoDatabaseError
+      else
+        raise
+      end
     end
   end
 end
@@ -73,11 +80,6 @@ def supports_extensions?
         false
       end
 
-      def supports_ranges?
-        # See cockroachdb/cockroach#17022
-        false
-      end
-
       def supports_materialized_views?
         false
       end
@@ -151,7 +153,7 @@ def initialize(connection, logger, conn_params, config)
         end
         @crdb_version = version_num
       end
-        
+
       private
 
         def initialize_type_map(m = type_map)

test/cases/adapter_test.rb

@@ -1,4 +1,8 @@
 require "cases/helper_cockroachdb"
+require "models/binary"
+require "models/developer"
+require "models/post"
+require "models/author"
 
 module CockroachDB
   class AdapterTest < ActiveRecord::TestCase
@@ -44,9 +48,86 @@ def test_remove_index_when_name_and_wrong_column_name_specified
     end
   end
 
+  class AdapterForeignKeyTest < ActiveRecord::TestCase
+    self.use_transactional_tests = false
+
+    fixtures :fk_test_has_pk
+
+    def before_setup
+      conn = ActiveRecord::Base.connection
+
+      conn.drop_table :fk_test_has_fk, if_exists: true
+      conn.drop_table :fk_test_has_pk, if_exists: true
+
+      conn.create_table :fk_test_has_pk, primary_key: "pk_id", force: :cascade do |t|
+      end
+
+      conn.create_table :fk_test_has_fk, force: true do |t|
+        t.references :fk, null: false
+        t.foreign_key :fk_test_has_pk, column: "fk_id", name: "fk_name", primary_key: "pk_id"
+      end
+
+      conn.execute "INSERT INTO fk_test_has_pk (pk_id) VALUES (1)"
+    end
+
+    def setup
+      @connection = ActiveRecord::Base.connection
+    end
+
+    def test_foreign_key_violations_are_translated_to_specific_exception_with_validate_false
+      klass_has_fk = Class.new(ActiveRecord::Base) do
+        self.table_name = "fk_test_has_fk"
+      end
+
+      error = assert_raises(ActiveRecord::InvalidForeignKey) do
+        has_fk = klass_has_fk.new
+        has_fk.fk_id = 1231231231
+        has_fk.save(validate: false)
+      end
+
+      assert_not_nil error.cause
+    end
+
+    # This is override to prevent an intermittent error
+    # Table fk_test_has_pk has constrain droped and not created back
+    def test_foreign_key_violations_on_insert_are_translated_to_specific_exception
+      error = assert_raises(ActiveRecord::InvalidForeignKey) do
+        insert_into_fk_test_has_fk
+      end
+
+      assert_not_nil error.cause
+    end
+
+    # This is override to prevent an intermittent error
+    # Table fk_test_has_pk has constrain droped and not created back
+    def test_foreign_key_violations_on_delete_are_translated_to_specific_exception
+      insert_into_fk_test_has_fk fk_id: 1
+
+      error = assert_raises(ActiveRecord::InvalidForeignKey) do
+        @connection.execute "DELETE FROM fk_test_has_pk WHERE pk_id = 1"
+      end
+
+      assert_not_nil error.cause
+    end
+
+    private
+
+    def insert_into_fk_test_has_fk(fk_id: 0)
+      # Oracle adapter uses prefetched primary key values from sequence and passes them to connection adapter insert method
+      if @connection.prefetch_primary_key?
+        id_value = @connection.next_sequence_value(@connection.default_sequence_name("fk_test_has_fk", "id"))
+        @connection.execute "INSERT INTO fk_test_has_fk (id,fk_id) VALUES (#{id_value},#{fk_id})"
+      else
+        @connection.execute "INSERT INTO fk_test_has_fk (fk_id) VALUES (#{fk_id})"
+      end
+    end
+  end
+
   class AdapterTestWithoutTransaction < ActiveRecord::TestCase
     self.use_transactional_tests = false
 
+    fixtures :posts, :authors, :author_addresses
+
     class Widget < ActiveRecord::Base
       self.primary_key = "widgetid"
     end
@@ -58,6 +139,7 @@ def setup
     teardown do
       @connection.drop_table :widgets, if_exists: true
       @connection.exec_query("DROP SEQUENCE IF EXISTS widget_seq")
+      @connection.exec_query("DROP SEQUENCE IF EXISTS widgets_seq")
     end
 
     # This test replaces the excluded test_reset_empty_table_with_custom_pk. We
@@ -74,5 +156,46 @@ def test_reset_empty_table_with_custom_pk_sequence
       ")
       assert_equal 1, Widget.create(name: "weather").id
     end
+
+    def test_truncate_tables
+      assert_operator Post.count, :>, 0
+      assert_operator Author.count, :>, 0
+      assert_operator AuthorAddress.count, :>, 0
+
+      @connection.truncate_tables("author_addresses", "authors", "posts")
+
+      assert_equal 0, Post.count
+      assert_equal 0, Author.count
+      assert_equal 0, AuthorAddress.count
+    ensure
+      reset_fixtures("author_addresses", "authors", "posts")
+    end
+
+    def test_truncate_tables_with_query_cache
+      @connection.enable_query_cache!
+
+      assert_operator Post.count, :>, 0
+      assert_operator Author.count, :>, 0
+      assert_operator AuthorAddress.count, :>, 0
+
+      @connection.truncate_tables("author_addresses", "authors", "posts")
+
+      assert_equal 0, Post.count
+      assert_equal 0, Author.count
+      assert_equal 0, AuthorAddress.count
+    ensure
+      reset_fixtures("author_addresses", "authors", "posts")
+      @connection.disable_query_cache!
+    end
+
+    private
+
+    def reset_fixtures(*fixture_names)
+      ActiveRecord::FixtureSet.reset_cache
+
+      fixture_names.each do |fixture_name|
+        ActiveRecord::FixtureSet.create_fixtures(FIXTURES_ROOT, fixture_name)
+      end
+    end
   end
 end

test/cases/adapters/postgresql/postgresql_adapter_test.rb

@@ -0,0 +1,25 @@
+require "cases/helper_cockroachdb"
+require "cases/helper"
+require "support/ddl_helper"
+require "support/connection_helper"
+
+module CockroachDB
+  module ConnectionAdapters
+    class PostgreSQLAdapterTest < ActiveRecord::PostgreSQLTestCase
+      self.use_transactional_tests = false
+      include DdlHelper
+      include ConnectionHelper
+
+      def setup
+        @connection = ActiveRecord::Base.connection
+        @connection_handler = ActiveRecord::Base.connection_handler
+      end
+
+      def test_database_exists_returns_false_when_the_database_does_not_exist
+        config = { database: "non_extant_database", adapter: "cockroachdb" }
+        assert_not ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.database_exists?(config),
+          "expected database #{config[:database]} to not exist"
+      end
+    end
+  end
+end

test/cases/associations/left_outer_join_association_test.rb

@@ -6,7 +6,7 @@
 
 module CockroachDB
   class LeftOuterJoinAssociationTest < ActiveRecord::TestCase
-    fixtures :authors, :posts
+    fixtures :author_addresses, :authors, :posts
 
     # This replaces the same test that's been excluded from
     # LeftOuterJoinAssociationTest. The query has been updated to guarantee the

test/cases/associations_test.rb

@@ -0,0 +1,108 @@
+require "cases/helper_cockroachdb"
+
+require "support/connection_helper"
+
+require "models/bird"
+require "models/parrot"
+require "models/pirate"
+require "models/price_estimate"
+require "models/ship"
+require "models/treasure"
+
+module CockroachDB
+  class WithAnnotationsTest < ActiveRecord::TestCase
+    self.use_transactional_tests = false
+
+    fixtures :pirates, :treasures, :parrots
+
+    def test_belongs_to_with_annotation_includes_a_query_comment
+      pirate = SpacePirate.where.not(parrot_id: nil).first
+      assert pirate, "should have a Pirate record"
+
+      log = capture_sql do
+        pirate.parrot
+      end
+      assert_not_predicate log, :empty?
+      assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty?
+
+      assert_sql(%r{/\* that tells jokes \*/}) do
+        pirate.parrot_with_annotation
+      end
+    end
+
+    def test_has_and_belongs_to_many_with_annotation_includes_a_query_comment
+      pirate = SpacePirate.first
+      assert pirate, "should have a Pirate record"
+
+      log = capture_sql do
+        pirate.parrots.first
+      end
+      assert_not_predicate log, :empty?
+      assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty?
+
+      assert_sql(%r{/\* that are very colorful \*/}) do
+        pirate.parrots_with_annotation.first
+      end
+    end
+
+    def test_has_one_with_annotation_includes_a_query_comment
+      pirate = SpacePirate.first
+      assert pirate, "should have a Pirate record"
+
+      log = capture_sql do
+        pirate.ship
+      end
+      assert_not_predicate log, :empty?
+      assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty?
+
+      assert_sql(%r{/\* that is a rocket \*/}) do
+        pirate.ship_with_annotation
+      end
+    end
+
+    def test_has_many_with_annotation_includes_a_query_comment
+      pirate = SpacePirate.first
+      assert pirate, "should have a Pirate record"
+
+      log = capture_sql do
+        pirate.birds.first
+      end
+      assert_not_predicate log, :empty?
+      assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty?
+
+      assert_sql(%r{/\* that are also parrots \*/}) do
+        pirate.birds_with_annotation.first
+      end
+    end
+
+    def test_has_many_through_with_annotation_includes_a_query_comment
+      pirate = SpacePirate.first
+      assert pirate, "should have a Pirate record"
+
+      log = capture_sql do
+        pirate.treasure_estimates.first
+      end
+      assert_not_predicate log, :empty?
+      assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty?
+
+      assert_sql(%r{/\* yarrr \*/}) do
+        pirate.treasure_estimates_with_annotation.first
+      end
+    end
+
+    def test_has_many_through_with_annotation_includes_a_query_comment_when_eager_loading
+      pirate = SpacePirate.first
+      assert pirate, "should have a Pirate record"
+
+      log = capture_sql do
+        pirate.treasure_estimates.first
+      end
+      assert_not_predicate log, :empty?
+      assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty?
+
+      assert_sql(%r{/\* yarrr \*/}) do
+        SpacePirate.includes(:treasure_estimates_with_annotation, :treasures).first
+      end
+    end
+  end
+end