bump activerecord to 8.0.0 (#356)

Cleanup useless code, update overriden methods.
Add a `#supports_native_partitioning? => false` method

Add timezone to quoted dates to comply with postgresql
way of parsing `timestamp with timezone`.

Add `#show_create` test.

Add test tool for excluding tests by name from
transactional tests.

Update CI.

Fixes #354

Author: BuonOmo

.github/workflows/ci.yml

@@ -40,8 +40,8 @@ jobs:
       fail-fast: false
       matrix:
         # https://www.cockroachlabs.com/docs/releases/release-support-policy
-        crdb: [v23.2, v24.1, v24.2]
-        ruby: ["3.3"]
+        crdb: [v23.2, v24.1, v24.3]
+        ruby: ["3.4"]
     name: Test (crdb=${{ matrix.crdb }} ruby=${{ matrix.ruby }})
     steps:
       - name: Set Up Actions

Gemfile

@@ -63,7 +63,7 @@ group :development, :test do
 
   # Gems used by the ActiveRecord test suite
   gem "bcrypt", "~> 3.1"
-  gem "sqlite3", "~> 1.4"
+  gem "sqlite3", "~> 2.1"
 
   gem "minitest", "~> 5.15"
 end

activerecord-cockroachdb-adapter.gemspec

@@ -14,9 +14,9 @@ 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", "~> 7.2.0"
+  spec.add_dependency "activerecord", "~> 8.0.0"
   spec.add_dependency "pg", "~> 1.5"
-  spec.add_dependency "rgeo-activerecord", "~> 7.0.0"
+  spec.add_dependency "rgeo-activerecord", "~> 8.0.0"
 
   spec.add_development_dependency "benchmark-ips", "~> 2.9.1"
 

lib/active_record/connection_adapters/cockroachdb/database_statements.rb

@@ -24,10 +24,8 @@ def insert_fixtures_set(fixture_set, tables_to_delete = [])
           table_deletes = tables_to_delete.map { |table| "DELETE FROM #{quote_table_name(table)}" }
           statements = table_deletes + fixture_inserts
 
-          with_multi_statements do
-            disable_referential_integrity do
-              execute_batch(statements, "Fixtures Load")
-            end
+          disable_referential_integrity do
+            execute_batch(statements, "Fixtures Load")
           end
         end
       end

lib/active_record/connection_adapters/cockroachdb/quoting.rb

@@ -50,6 +50,14 @@ def quote(value)
             super
           end
         end
+
+        def quoted_date(value)
+          # CockroachDB differs from PostgreSQL in its representation of
+          # a `timestamp with timezone`, it does not always include the
+          # timezone offset (e.g. `+00`), so we need to add it here.
+          # This is tested by `BasicsTest#test_default_in_local_time`.
+          super + value.strftime("%z")
+        end
       end
     end
   end

lib/active_record/connection_adapters/cockroachdb_adapter.rb

@@ -161,6 +161,15 @@ def supports_nulls_not_distinct?
         false
       end
 
+      # Partitioning is quite different from PostgreSQL, so we don't support it.
+      # If you need partitioning, you should default to using raw SQL queries.
+      #
+      # See https://www.postgresql.org/docs/current/ddl-partitioning.html
+      # See https://www.cockroachlabs.com/docs/stable/partitioning
+      def supports_native_partitioning?
+        false
+      end
+
       def supports_ddl_transactions?
         false
       end
@@ -466,9 +475,7 @@ def crdb_column_definitions(table_name)
         # That method will not work for CockroachDB because the error
         # originates from the "runExecBuilder" function, so we need
         # to modify the original to match the CockroachDB behavior.
-        def is_cached_plan_failure?(e)
-          pgerror = e.cause
-
+        def is_cached_plan_failure?(pgerror)
           pgerror.result.result_error_field(PG::PG_DIAG_SQLSTATE) == FEATURE_NOT_SUPPORTED &&
             pgerror.result.result_error_field(PG::PG_DIAG_SOURCE_FUNCTION) == "runExecBuilder"
         rescue

lib/active_record/relation/query_methods_ext.rb

@@ -57,7 +57,7 @@ def force_index(index_name, direction: nil)
       def force_index!(index_name, direction: nil)
         return self unless from_clause_is_a_table_name?
 
-        index_name = sanitize_sql(index_name.to_s)
+        index_name = model.sanitize_sql(index_name.to_s)
         direction = direction.to_s.upcase
         direction = %w[ASC DESC].include?(direction) ? ",#{direction}" : ""
 
@@ -84,7 +84,7 @@ def index_hint(hint)
       def index_hint!(hint)
         return self unless from_clause_is_a_table_name?
 
-        hint = sanitize_sql(hint.to_s)
+        hint = model.sanitize_sql(hint.to_s)
         @index_hint = hint.to_s
         self.from_clause = build_from_clause_with_hints
         self
@@ -120,7 +120,7 @@ def build_from_clause_with_hints
 
         table_name =
           if from_clause.empty?
-            quoted_table_name
+            model.quoted_table_name
           else
             # Remove previous table hints if any. And spaces.
             from_clause.value.partition("@").first.strip

test/cases/helper_cockroachdb.rb

@@ -1,7 +1,11 @@
 require 'bundler'
 Bundler.setup
 
-CRDB_VALIDATE_BUG = "CockcroachDB bug, see https://github.com/cockroachdb/cockroach/blob/dd1e0e0164cb3d5859ea4bb23498863d1eebc0af/pkg/sql/alter_table.go#L458-L467"
+module ExcludeMessage
+  VALIDATE_BUG = "CockcroachDB bug, see https://github.com/cockroachdb/cockroach/blob/dd1e0e0164cb3d5859ea4bb23498863d1eebc0af/pkg/sql/alter_table.go#L458-L467"
+  NO_HSTORE = "Extension \"hstore\" is not yet supported by CRDB. See https://github.com/cockroachdb/cockroach/issues/41284"
+end
+
 require "minitest/excludes"
 require "minitest/github_action_reporter"
 
@@ -17,24 +21,13 @@
 # and COCKROACH_SKIP_LOAD_SCHEMA that can
 # skip this step
 require "support/load_schema_helper"
-class NoPGSchemaTestCase < SimpleDelegator
-  def current_adapter?(...)
-    false
-  end
-end
 
 module LoadSchemaHelperExt
   def load_schema
-    # TODO: Remove this const_set mess once https://github.com/rails/rails/commit/d5c2ff8345c9d23b7326edb2bbe72b6e86a63140
-    #   is part of a rails release.
-    old_helper = ActiveRecord::TestCase
-    ActiveRecord.const_set(:TestCase, NoPGSchemaTestCase.new(ActiveRecord::TestCase))
     return if ENV['COCKROACH_LOAD_FROM_TEMPLATE']
     return if ENV['COCKROACH_SKIP_LOAD_SCHEMA']
 
     super
-  ensure
-    ActiveRecord.const_set(:TestCase, old_helper)
   end
 end
 LoadSchemaHelper.prepend(LoadSchemaHelperExt)
@@ -44,6 +37,11 @@ def load_schema
 # Load ActiveRecord test helper
 require "cases/helper"
 
+require "support/exclude_from_transactional_tests"
+
+# Allow exclusion of tests by name using #exclude_from_transactional_tests(test_name)
+ActiveRecord::TestCase.prepend(ExcludeFromTransactionalTests)
+
 # Load the CockroachDB specific schema. It replaces ActiveRecord's PostgreSQL
 # specific schema.
 def load_cockroachdb_specific_schema

test/cases/migration/check_constraint_test.rb

@@ -37,7 +37,7 @@ def test_validate_check_constraint_by_name
             end
           end
 
-          # CRDB_VALIDATE_BUG
+          # ExcludeMessage::VALIDATE_BUG
           # def test_schema_dumping_with_validate_false
           #   @connection.add_check_constraint :trades, "quantity > 0", name: "quantity_check", validate: false
 

test/cases/show_create_test.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+require "cases/helper_cockroachdb"
+require "models/post"
+
+module CockroachDB
+  class ShowCreateTest < ActiveRecord::TestCase
+    fixtures :posts
+
+    def test_show_create
+      assert_match(/CREATE TABLE public\.posts/, Post.show_create)
+    end
+  end
+end