bump activerecord to 8.0.0

.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

test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapter/QuotingTest.rb

@@ -4,3 +4,19 @@
 exclude :test_do_not_raise_when_int_is_not_wider_than_64bit, comment
 exclude :test_do_not_raise_when_raise_int_wider_than_64bit_is_false, comment
 exclude :test_raise_when_int_is_wider_than_64bit, comment
+
+require "support/copy_cat"
+
+CopyCat.copy_methods(self, self,
+  :test_quote_big_decimal,
+  :test_quote_rational,
+  :test_quote_integer) do
+  def on_str(node)
+    return if defined?(@already_quoted)
+    @already_quoted = true
+    node => [:str, str]
+    return unless ["4.2", "3/4", "42"].include?(str)
+
+    replace(node.loc.expression, "'#{str}'".inspect)
+  end
+end

test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapterTest.rb

@@ -33,3 +33,8 @@
 exclude :test_allowlist_of_warning_codes_to_ignore, plpgsql_needed
 
 exclude :test_translate_no_connection_exception_to_not_established, "CRDB doesn't implement pg_terminate_backend()"
+
+exclude :test_disable_extension_without_schema, ExcludeMessage::NO_HSTORE
+exclude :test_extensions_omits_current_schema_name, ExcludeMessage::NO_HSTORE
+exclude :test_extensions_includes_non_current_schema_name, ExcludeMessage::NO_HSTORE
+exclude :test_disable_extension_with_schema, ExcludeMessage::NO_HSTORE

test/excludes/ActiveRecord/ConnectionAdapters/RegistrationIsolatedTest.rb

@@ -0,0 +1,14 @@
+exclude "test_#resolve_raises_if_the_adapter_is_using_the_pre_7.2_adapter_registration_API", "One adapter is missing in the test, added below"
+
+test "#resolve raises if the adapter is using the pre 7.2 adapter registration API with CRDB" do
+  exception = assert_raises(ActiveRecord::AdapterNotFound) do
+    ActiveRecord::ConnectionAdapters.resolve("fake_legacy")
+  end
+
+  assert_equal(
+    "Database configuration specifies nonexistent 'fake_legacy' adapter. Available adapters are: abstract, cockroachdb, fake, mysql2, postgresql, sqlite3, trilogy. Ensure that the adapter is spelled correctly in config/database.yml and that you've added the necessary adapter gem to your Gemfile if it's not in the list of available adapters.",
+    exception.message
+  )
+ensure
+  ActiveRecord::ConnectionAdapters.instance_variable_get(:@adapters).delete("fake_legacy")
+end

test/excludes/ActiveRecord/Migration/CompositeForeignKeyTest.rb

@@ -1,2 +1,2 @@
 exclude :test_add_composite_foreign_key_raises_without_options, "Updated regexp to remove quotes"
-exclude :test_schema_dumping, CRDB_VALIDATE_BUG
+exclude :test_schema_dumping, ExcludeMessage::VALIDATE_BUG

test/excludes/ActiveRecord/Migration/ForeignKeyTest.rb

@@ -18,10 +18,10 @@
 exclude :test_validate_foreign_key_by_name, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass."
 exclude :test_validate_constraint_by_name, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass."
 exclude :test_schema_dumping, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass."
-exclude :test_schema_dumping_on_delete_and_on_update_options, CRDB_VALIDATE_BUG
-exclude :test_schema_dumping_with_validate_false, CRDB_VALIDATE_BUG
-exclude :test_schema_dumping_with_validate_true, CRDB_VALIDATE_BUG
-exclude :test_schema_dumping_with_custom_fk_ignore_pattern, CRDB_VALIDATE_BUG
+exclude :test_schema_dumping_on_delete_and_on_update_options, ExcludeMessage::VALIDATE_BUG
+exclude :test_schema_dumping_with_validate_false, ExcludeMessage::VALIDATE_BUG
+exclude :test_schema_dumping_with_validate_true, ExcludeMessage::VALIDATE_BUG
+exclude :test_schema_dumping_with_custom_fk_ignore_pattern, ExcludeMessage::VALIDATE_BUG
 exclude :test_add_foreign_key_is_reversible, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass."
 exclude :test_foreign_key_constraint_is_not_cached_incorrectly, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass."
 exclude :test_add_foreign_key_with_prefix, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass."

test/excludes/EachTest.rb

@@ -0,0 +1,8 @@
+require "support/copy_cat"
+
+# CockroachDB doesn't update schema information when adding an
+# index until the transaction is done. Hence impossible to delete
+# this index before completion of the transaction.
+exclude_from_transactional_tests :test_in_batches_iterating_using_custom_columns
+exclude_from_transactional_tests :test_in_batches_with_custom_columns_raises_when_non_unique_columns
+exclude_from_transactional_tests :test_in_batches_when_loaded_iterates_using_custom_column

test/excludes/FixturesTest.rb

@@ -7,3 +7,16 @@
 exclude :test_bulk_insert_with_a_multi_statement_query_raises_an_exception_when_any_insert_fails, "Skipping the PostgreSQL test, but reimplemented for CockroachDB in test/cases/fixtures_test.rb"
 exclude :test_inserts_with_pre_and_suffix, "Skipping the PostgreSQL test, but reimplemented for CockroachDB in test/cases/fixtures_test.rb"
 exclude :test_create_symbol_fixtures, "Skipping the PostgreSQL test, but reimplemented for CockroachDB in test/cases/fixtures_test.rb"
+
+module Replacement
+  def test_insert_with_default_function
+    before = Time.now.utc
+    create_fixtures("aircrafts")
+    after = Time.now.utc
+
+    aircraft = Aircraft.find_by(name: "boeing-with-no-manufactured-at")
+    assert before <= aircraft.manufactured_at && aircraft.manufactured_at <= after
+  end
+end
+
+prepend Replacement

test/excludes/MultiDbMigratorTest.rb

@@ -1,10 +1,2 @@
-require "support/copy_cat"
-
-class CockroachDB < MultiDbMigratorTest
-	self.use_transactional_tests = false
-
-	CopyCat.copy_methods(self, MultiDbMigratorTest, :test_internal_metadata_stores_environment)
-end
-
-exclude :test_internal_metadata_stores_environment, "We can't add " \
-	"and remove a column in the same transaction with CockroachDB"
+# We can't add and remove a column in the same transaction with CockroachDB
+exclude_from_transactional_tests :test_internal_metadata_stores_environment

test/excludes/PostgresqlExtensionMigrationTest.rb

@@ -1,6 +1,5 @@
 exclude :test_disable_extension_migration_ignores_prefix_and_suffix, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_enable_extension_migration_ignores_prefix_and_suffix, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-no_hstore = "Extension \"hstore\" is not yet supported by CRDB"
-exclude :test_disable_extension_drops_extension_when_cascading, no_hstore
-exclude :test_disable_extension_raises_when_dependent_objects_exist, no_hstore
-exclude :test_enable_extension_migration_with_schema, no_hstore
+exclude :test_disable_extension_drops_extension_when_cascading, ExcludeMessage::NO_HSTORE
+exclude :test_disable_extension_raises_when_dependent_objects_exist, ExcludeMessage::NO_HSTORE
+exclude :test_enable_extension_migration_with_schema, ExcludeMessage::NO_HSTORE

test/excludes/PostgresqlHstoreTest.rb

@@ -37,9 +37,9 @@
 exclude :test_schema_dump_with_shorthand, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_dirty_from_user_equal, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_hstore_dirty_from_database_equal, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_changes_with_store_accessors, "Skipping because the test uses hstore. See https://github.com/cockroachdb/cockroach/issues/41284"
-exclude :test_various_null, "Skipping because the test uses hstore. See https://github.com/cockroachdb/cockroach/issues/41284"
-exclude :test_spaces, "Skipping because the test uses hstore. See https://github.com/cockroachdb/cockroach/issues/41284"
-exclude :test_signs, "Skipping because the test uses hstore. See https://github.com/cockroachdb/cockroach/issues/41284"
-exclude :test_equal_signs, "Skipping because the test uses hstore. See https://github.com/cockroachdb/cockroach/issues/41284"
-exclude :test_commas, "Skipping because the test uses hstore. See https://github.com/cockroachdb/cockroach/issues/41284"
+exclude :test_changes_with_store_accessors, ExcludeMessage::NO_HSTORE
+exclude :test_various_null, ExcludeMessage::NO_HSTORE
+exclude :test_spaces, ExcludeMessage::NO_HSTORE
+exclude :test_signs, ExcludeMessage::NO_HSTORE
+exclude :test_equal_signs, ExcludeMessage::NO_HSTORE
+exclude :test_commas, ExcludeMessage::NO_HSTORE

test/excludes/PostgresqlPointTest.rb

@@ -12,3 +12,4 @@
 exclude :test_legacy_schema_dumping, "Geometric types are not supported in CockroachDB. See https://github.com/cockroachdb/cockroach/issues/21286."
 exclude :test_legacy_roundtrip, "Geometric types are not supported in CockroachDB. See https://github.com/cockroachdb/cockroach/issues/21286."
 exclude :test_legacy_mutation, "Geometric types are not supported in CockroachDB. See https://github.com/cockroachdb/cockroach/issues/21286."
+exclude :test_hash_assignment, "Geometric types are not supported in CockroachDB. See https://github.com/cockroachdb/cockroach/issues/21286."

test/excludes/SchemaCreateTableOptionsTest.rb

@@ -0,0 +1,2 @@
+exclude :test_multiple_inherited_table_options_is_dumped, "INHERITS keyword unsupported. See https://go.crdb.dev/issue-v/22456/v24.3"
+exclude :test_inherited_table_options_is_dumped, "INHERITS keyword unsupported. See https://go.crdb.dev/issue-v/22456/v24.3"

test/excludes/SchemaForeignKeyTest.rb

@@ -1 +1 @@
-exclude :test_dump_foreign_key_targeting_different_schema, CRDB_VALIDATE_BUG
+exclude :test_dump_foreign_key_targeting_different_schema, ExcludeMessage::VALIDATE_BUG

test/schema/cockroachdb_specific_schema.rb

@@ -5,8 +5,8 @@
 # with an explanation of why they don't work.
 
 ActiveRecord::Schema.define do
-  ActiveRecord::TestCase.enable_extension!("uuid-ossp", ActiveRecord::Base.lease_connection)
-  ActiveRecord::TestCase.enable_extension!("pgcrypto",  ActiveRecord::Base.lease_connection) if ActiveRecord::Base.lease_connection.supports_pgcrypto_uuid?
+  ActiveRecord::TestCase.enable_extension!("uuid-ossp", connection)
+  ActiveRecord::TestCase.enable_extension!("pgcrypto",  connection) if connection.supports_pgcrypto_uuid?
 
   uuid_default = connection.supports_pgcrypto_uuid? ? {} : { default: "uuid_generate_v4()" }
 
@@ -45,6 +45,7 @@
     t.string :char2, limit: 50, default: "a varchar field"
     t.text :char3, default: "a text field"
     t.bigint :bigint_default, default: -> { "0::bigint" }
+    t.binary :binary_default_function, default: -> { "convert_to('A', 'UTF8')" }
     t.text :multiline_default, default: "--- []
 
 "
@@ -177,11 +178,13 @@
     t.integer :position_1
     t.integer :position_2
     t.integer :position_3
+    t.integer :position_4
 
-    # CockroachDB does not support deferrable, hence these three lines have been simplified.
+    # CockroachDB does not support deferrable, hence these four lines have been simplified.
     t.unique_constraint :position_1, name: "test_unique_constraints_position_1"
     t.unique_constraint :position_2, name: "test_unique_constraints_position_2"
     t.unique_constraint :position_3, name: "test_unique_constraints_position_3"
+    t.unique_constraint :position_4, name: "test_unique_constraints_position_4"
   end
 
   if supports_partitioned_indexes?
@@ -200,6 +203,9 @@
 
   add_index(:companies, [:firm_id, :type], name: "company_include_index", include: [:name, :account_id])
 
+  # In the original PostgreSQL schema, there would be a table here, populated using triggers.
+  # This is not supported by Cockroachdb so we removed that bit.
+
   create_table :buildings, force: true do |t|
     t.st_point :coordinates, srid: 3857
     t.st_point :latlon, srid: 4326, geographic: true