feat: upgrade to Rails 8.1

.github/workflows/flaky.yml

@@ -33,15 +33,16 @@ jobs:
           ruby=$(jq --raw-input --compact-output 'split(" ")' <<<"${{ github.event.inputs.ruby }}")
           crdb_len=$(wc -w <<<"${{ github.event.inputs.crdb }}")
           ruby_len=$(wc -w <<<"${{ github.event.inputs.ruby }}")
-          (( range_count = ${{github.event.inputs.max}} / ( crdb_len * ruby_len ) ))
-          range=$(jq --compact-output "[range($range_count)]" <<<[])
+          (( seeds_count = ${{github.event.inputs.max}} / ( crdb_len * ruby_len ) ))
+          seeds=$(shuf --input-range=1-65535 --head-count=$seeds_count | jq --slurp --compact-output)
+          echo $seeds
           echo "crdb=$crdb" >> $GITHUB_OUTPUT
           echo "ruby=$ruby" >> $GITHUB_OUTPUT
-          echo "numbers=$range" >> $GITHUB_OUTPUT
+          echo "seeds=$seeds" >> $GITHUB_OUTPUT
     outputs:
       crdb: ${{ steps.generate-matrix.outputs.crdb }}
       ruby: ${{ steps.generate-matrix.outputs.ruby }}
-      numbers: ${{ steps.generate-matrix.outputs.numbers }}
+      seeds: ${{ steps.generate-matrix.outputs.seeds }}
   test:
     runs-on: ubuntu-latest
     needs: prepare-matrix
@@ -50,13 +51,23 @@ jobs:
       matrix:
         crdb: ${{ fromJSON(needs.prepare-matrix.outputs.crdb) }}
         ruby: ${{ fromJSON(needs.prepare-matrix.outputs.ruby) }}
-        number: ${{ fromJSON(needs.prepare-matrix.outputs.numbers) }}
-    name: Test (crdb=${{ matrix.crdb }} ruby=${{ matrix.ruby }} number=${{ matrix.number }})
+        seed: ${{ fromJSON(needs.prepare-matrix.outputs.seeds) }}
+    name: Test (crdb=${{ matrix.crdb }} ruby=${{ matrix.ruby }} seed=${{ matrix.seed }})
+    env:
+      SEED: ${{ matrix.seed }}
+      COCKROACH_LOAD_FROM_TEMPLATE: yes_please
     steps:
       - name: Set Up Actions
         uses: actions/checkout@v4
       - uses: ./.github/actions/test-runner
+        id: test
         with:
           crdb: ${{ matrix.crdb }}
           ruby: ${{ matrix.ruby }}
           TESTOPTS: --fail-fast
+      - name: Bisect failing test
+        if: ${{ failure() && steps.test.conclusion == 'failure' }}
+        run: bin/minitest_bisect ${{ matrix.seed }}
+        env:
+          MTB_VERBOSE: 2
+          MTB_DEBUG: 1

CONTRIBUTING.md

@@ -61,23 +61,16 @@ Only do it if you know the schema was left in a correct state.
 
 ### Run Tests from a Backup
 
-Loading the full test schema every time a test runs can take a while, so for cases where loading the schema sequentially is unimportant, it is possible to use a backup to set up the database. This is significantly faster than the standard method and is provided to run individual tests faster, but should not be used to validate a build.
-
-First create the template database.
-
-```bash
-bundle exec rake db:create_test_template
-```
-
-This will create a template database for the current version (ex. `activerecord_test_template611` for version 6.1.1) and create a `BACKUP` in the `nodelocal://self/activerecord-crdb-adapter/#{activerecord_version}` directory.
-
-To load from the template, use the `COCKROACH_LOAD_FROM_TEMPLATE` flag.
-
-```bash
-COCKROACH_LOAD_FROM_TEMPLATE=1 TEST_FILES="test/cases/adapters/postgresql/ddl_test.rb" bundle exec rake test
-```
-
-And the `activerecord_unittest` database will use the `RESTORE` command to load the schema from the template database.
+Loading the full test schema every time a test runs can take
+a while, so for cases where loading the schema sequentially
+is unimportant, it is possible to use a backup to set up the
+database. This is significantly faster than the standard
+method and is provided to run individual tests faster, but
+should not be used to validate a build.
+
+To do so, just set the env variable `COCKROACH_LOAD_FROM_TEMPLATE`.
+First run will generate and cache a template, latter runs will use
+it.
 
 # Improvements
 

Gemfile

@@ -54,6 +54,7 @@ group :development, :test do
 
   gem "rake"
   gem "debug"
+  gem "minitest-bisect", github: "BuonOmo/minitest-bisect", branch: "no-fail-fast"
   gem "minitest-excludes", "~> 2.0.1"
   gem "minitest-github_action_reporter", github: "BuonOmo/minitest-github_action_reporter", require: "minitest/github_action_reporter_plugin"
   gem "ostruct", "~> 0.6"

Rakefile

@@ -2,28 +2,9 @@ require "bundler/gem_tasks"
 require "rake/testtask"
 require_relative 'test/support/paths_cockroachdb'
 require_relative 'test/support/rake_helpers'
-require_relative 'test/support/template_creator'
 
 task default: [:test]
 
-namespace :db do
-  task "create_test_template" do
-    ENV['DEBUG_COCKROACHDB_ADAPTER'] = "1"
-    ENV['COCKROACH_SKIP_LOAD_SCHEMA'] = "1"
-
-    TemplateCreator.connect
-    require_relative 'test/cases/helper'
-
-    # TODO: look into this more, but for some reason the blob alias
-    # is not defined while running this task.
-    ActiveRecord::ConnectionAdapters::CockroachDB::TableDefinition.class_eval do
-      alias :blob :binary
-    end
-
-    TemplateCreator.create_test_template
-  end
-end
-
 Rake::TestTask.new do |t|
   t.libs = ARTest::CockroachDB.test_load_paths
   t.test_files = RakeHelpers.test_files

activerecord-cockroachdb-adapter.gemspec

@@ -14,7 +14,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", "~> 8.0.0"
+  spec.add_dependency "activerecord", "~> 8.1.0.a"
   spec.add_dependency "pg", "~> 1.5"
   spec.add_dependency "rgeo-activerecord", "~> 8.0.0"
 

bin/minitest_bisect

@@ -0,0 +1,19 @@
+#!/usr/bin/env -S bundle exec ruby
+
+require 'rake/file_list'
+require_relative '../test/support/paths_cockroachdb'
+require_relative '../test/support/rake_helpers'
+
+libs = ARTest::CockroachDB.test_load_paths
+test_files = RakeHelpers.test_files
+
+Dir.chdir(File.dirname __dir__) do
+  system(
+    "bundle",
+    "exec",
+  	"minitest_bisect",
+  	"--seed=#{ARGV[0]}",
+  	"-I" + libs.join(":"),
+  	*test_files
+  )
+end

lib/active_record/connection_adapters/cockroachdb/column.rb

@@ -20,7 +20,7 @@ module CockroachDB
       class Column < PostgreSQL::Column
         # most functions taken from activerecord-postgis-adapter spatial_column
         # https://github.com/rgeo/activerecord-postgis-adapter/blob/master/lib/active_record/connection_adapters/postgis/spatial_column.rb
-        def initialize(name, default, sql_type_metadata = nil, null = true,
+        def initialize(name, cast_type, default, sql_type_metadata = nil, null = true,
                        default_function = nil, collation: nil, comment: nil, identity: nil,
                        serial: nil, spatial: nil, generated: nil, hidden: nil)
           @sql_type_metadata = sql_type_metadata
@@ -45,7 +45,7 @@ def initialize(name, default, sql_type_metadata = nil, null = true,
             # @geometric_type = geo_type_from_sql_type(sql_type)
             build_from_sql_type(sql_type_metadata.sql_type)
           end
-          super(name, default, sql_type_metadata, null, default_function,
+          super(name, cast_type, default, sql_type_metadata, null, default_function,
             collation: collation, comment: comment, serial: serial, generated: generated, identity: identity)
           if spatial? && @srid
             @limit = { srid: @srid, type: to_type_name(geometric_type) }

lib/active_record/connection_adapters/cockroachdb/database_tasks.rb

@@ -74,7 +74,7 @@ def structure_load(filename, extra_flags=nil)
               "https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/new"
           end
 
-          run_cmd("cockroach", ["sql", "--set", "errexit=false", "--file", filename], "loading")
+          run_cmd("cockroach", "sql", "--set", "errexit=false", "--file", filename)
         end
 
         private

lib/active_record/connection_adapters/cockroachdb/oid/spatial.rb

@@ -28,6 +28,10 @@ class Spatial < Type::Value
           def initialize(oid, sql_type)
             @sql_type = sql_type
             @geo_type, @srid, @has_z, @has_m = self.class.parse_sql_type(sql_type)
+            @spatial_factory =
+              RGeo::ActiveRecord::SpatialFactoryStore.instance.factory(
+                factory_attrs
+              )
           end
 
           # sql_type: geometry, geometry(Point), geometry(Point,4326), ...
@@ -59,13 +63,6 @@ def self.parse_sql_type(sql_type)
             [geo_type, srid, has_z, has_m]
           end
 
-          def spatial_factory
-            @spatial_factory ||=
-              RGeo::ActiveRecord::SpatialFactoryStore.instance.factory(
-                factory_attrs
-              )
-          end
-
           def geographic?
             @sql_type =~ /geography/
           end
@@ -108,14 +105,14 @@ def parse_wkt(string)
           end
 
           def binary_string?(string)
-            string[0] == "\x00" || string[0] == "\x01" || string[0, 4] =~ /[0-9a-fA-F]{4}/
+            string[0] == "\x00" || string[0] == "\x01" || string[0, 4].match?(/[0-9a-fA-F]{4}/)
           end
 
           def wkt_parser(string)
             if binary_string?(string)
-              RGeo::WKRep::WKBParser.new(spatial_factory, support_ewkb: true, default_srid: @srid)
+              RGeo::WKRep::WKBParser.new(@spatial_factory, support_ewkb: true, default_srid: @srid)
             else
-              RGeo::WKRep::WKTParser.new(spatial_factory, support_ewkt: true, default_srid: @srid)
+              RGeo::WKRep::WKTParser.new(@spatial_factory, support_ewkt: true, default_srid: @srid)
             end
           end
 

lib/active_record/connection_adapters/cockroachdb/schema_statements.rb

@@ -250,6 +250,7 @@ def new_column_from_field(table_name, field, _definition)
 
           CockroachDB::Column.new(
             column_name,
+            get_oid_type(oid.to_i, fmod.to_i, column_name, type),
             default_value,
             type_metadata,
             !notnull,

lib/active_record/connection_adapters/cockroachdb_adapter.rb

@@ -299,7 +299,7 @@ def initialize_type_map(m = type_map)
             st_polygon
           ).each do |geo_type|
             m.register_type(geo_type) do |oid, _, sql_type|
-              CockroachDB::OID::Spatial.new(oid, sql_type)
+              CockroachDB::OID::Spatial.new(oid, sql_type).freeze
             end
           end
 

test/cases/adapter_test.rb

@@ -70,6 +70,7 @@ class AdapterForeignKeyTest < ActiveRecord::TestCase
     fixtures :fk_test_has_pk
 
     def before_setup
+      super
       conn = ActiveRecord::Base.lease_connection
 
       conn.drop_table :fk_test_has_fk, if_exists: true
@@ -87,6 +88,7 @@ def before_setup
     end
 
     def setup
+      super
       @connection = ActiveRecord::Base.lease_connection
     end
 

test/cases/adapters/postgresql/postgis_test.rb

@@ -185,12 +185,6 @@ def reset_memoized_spatial_factories
     # necessary to reset the @spatial_factory variable on spatial
     # OIDs, otherwise the results of early tests will be memoized
     # since the table is not dropped and recreated between test cases.
-    ObjectSpace.each_object(spatial_oid) do |oid|
-      oid.instance_variable_set(:@spatial_factory, nil)
-    end
-  end
-
-  def spatial_oid
-    ActiveRecord::ConnectionAdapters::CockroachDB::OID::Spatial
+    klass.lease_connection.send(:reload_type_map)
   end
 end

test/cases/fixtures_test.rb

@@ -18,6 +18,29 @@
 require "models/traffic_light"
 require "models/treasure"
 
+# Hacky tool that searches for table definition code in schema.rb
+# and evals it. Doing this ensure we're always using the latest
+# schema.
+module TableCreator
+  def self.create_table_using_test_schema(table_name, connection)
+    table_name = table_name.to_sym
+    @schema_file ||= SCHEMA_ROOT + "/schema.rb"
+    @ast ||= Prism::Translation::Parser.parse_file(@schema_file)
+    to_search = [@ast]
+    found = nil
+    while !to_search.empty?
+      node = to_search.shift
+      next unless node.is_a?(Parser::AST::Node)
+      if node in [:block, [:send, _, :create_table, [:sym, ^table_name], *], *]
+        break found = node
+      end
+      to_search += node.children
+    end
+    raise "Schema #{table_name.inspect} not found" unless found
+    connection.instance_eval(found.location.expression.source)
+  end
+end
+
 module CockroachDB
   class FixturesTest < ActiveRecord::TestCase
     include ConnectionHelper
@@ -288,6 +311,7 @@ class FixturesResetPkSequenceTest < ActiveRecord::TestCase
     # We'll do this in a before_setup so we get ahead of
     # ActiveRecord::TestFixtures#before_setup.
     def before_setup
+      super
       Account.lease_connection.drop_table :accounts, if_exists: true
       Account.lease_connection.exec_query("CREATE SEQUENCE IF NOT EXISTS accounts_id_seq")
       Account.lease_connection.exec_query("
@@ -336,53 +360,33 @@ def before_setup
     end
 
     def setup
+      super
       @instances = [Account.new(credit_limit: 50), Company.new(name: "RoR Consulting"), Course.new(name: "Test")]
       ActiveRecord::FixtureSet.reset_cache # make sure tables get reinitialized
     end
 
     # Drop the primary key sequences and bring back the original tables.
     def teardown
+      super
       Account.lease_connection.drop_table :accounts, if_exists: true
       Account.lease_connection.exec_query("DROP SEQUENCE IF EXISTS accounts_id_seq")
-      Account.lease_connection.create_table :accounts, force: true do |t|
-        t.timestamps null: true
-        t.references :firm, index: false
-        t.string  :firm_name
-        t.integer :credit_limit
-        t.string :status
-        t.integer "a" * max_identifier_length
-      end
+      TableCreator.create_table_using_test_schema(:accounts, Account.lease_connection)
 
       Company.lease_connection.drop_table :companies, if_exists: true
       Company.lease_connection.exec_query("DROP SEQUENCE IF EXISTS companies_nonstd_seq CASCADE")
-      Company.lease_connection.create_table :companies, force: true do |t|
-        t.string :type
-        t.references :firm, index: false
-        t.string  :firm_name
-        t.string  :name
-        t.bigint :client_of
-        t.bigint :rating, default: 1
-        t.integer :account_id
-        t.string :description, default: ""
-        t.integer :status, default: 0
-        t.index [:name, :rating], order: :desc
-        t.index [:name, :description], length: 10
-        t.index [:firm_id, :type, :rating], name: "company_index", length: { type: 10 }, order: { rating: :desc }
-        t.index [:firm_id, :type], name: "company_partial_index", where: "(rating > 10)"
-        t.index [:firm_id], name: "company_nulls_not_distinct", nulls_not_distinct: true
-        t.index :name, name: "company_name_index", using: :btree
-        t.index "(CASE WHEN rating > 0 THEN lower(name) END) DESC", name: "company_expression_index" if Company.lease_connection.supports_expression_index?
-        t.index [:firm_id, :type], name: "company_include_index", include: [:name, :account_id]
-      end
+      TableCreator.create_table_using_test_schema(:companies, Company.lease_connection)
+      # CockroachDB specific schema addition
+      Company.lease_connection.add_index(:companies, [:firm_id, :type], name: "company_include_index", include: [:name, :account_id])
 
       Course.lease_connection.drop_table :courses, if_exists: true
       Course.lease_connection.exec_query("DROP SEQUENCE IF EXISTS courses_id_seq")
-      Course.lease_connection.create_table :courses, force: true do |t|
-        t.column :name, :string, null: false
-        t.column :college_id, :integer, index: true
-      end
+      TableCreator.create_table_using_test_schema(:courses, Course.lease_connection)
 
       recreate_parrots
+
+      Account.reset_column_information
+      Company.reset_column_information
+      Course.reset_column_information
     end
 
     # This replaces the same test that's been excluded from
@@ -448,28 +452,9 @@ def recreate_parrots
         conn.drop_table :parrots_treasures, if_exists: true
         conn.drop_table :parrots, if_exists: true
 
-        conn.create_table :parrots, force: :cascade do |t|
-          t.string :name
-          t.integer :breed, default: 0
-          t.string :color
-          t.string :parrot_sti_class
-          t.integer :killer_id
-          t.integer :updated_count, :integer, default: 0
-          t.datetime :created_at
-          t.datetime :created_on
-          t.datetime :updated_at
-          t.datetime :updated_on
-        end
-
-        conn.create_table :parrots_pirates, id: false, force: true do |t|
-          t.references :parrot, foreign_key: true
-          t.references :pirate, foreign_key: true
-        end
-
-        conn.create_table :parrots_treasures, id: false, force: true do |t|
-          t.references :parrot, foreign_key: true
-          t.references :treasure, foreign_key: true
-        end
+        TableCreator.create_table_using_test_schema(:parrots, conn)
+        TableCreator.create_table_using_test_schema(:parrots_pirates, conn)
+        TableCreator.create_table_using_test_schema(:parrots_treasures, conn)
       end
     end
   end