feat: upgrade to Rails 8.1

.github/workflows/ci.yml

@@ -50,23 +50,44 @@ jobs:
           <table>
           <thead>
             <tr>
-              <th></th>
+              <th>#</th>
               <th>Test</th>
+              <th>Failure</th>
             </tr>
           </thead>
           <tbody>
           EOF
 
           jq --slurp --raw-output '
-            map(.failed_tests|map("\(.klass)#\(.NAME)")) | flatten | unique | sort | to_entries[]
-            | "<tr><td><strong>\(.key)</strong></td><td>\(.value)</td></tr>"
+            map(.failed_tests) | flatten | map({
+              klass,
+              NAME,
+              failure: .failures[0],
+              source_url: (
+                .source_location | if (.[0] | contains("/gems/")) then
+                  (.[0] | capture("rails-(?<sha>.*?)/(?<path>.*)")) *
+                  {line: .[1], server: "https://github.com", repo: "rails/rails"}
+                else
+                  (.[0] | capture("activerecord-cockroachdb-adapter/(?<path>test/.*)") *
+                  {line: .[1], sha: $ENV.GITHUB_SHA, repo: $ENV.GITHUB_REPOSITORY, server: $ENV.GITHUB_SERVER_URL}
+                end | "\(.server)/\(.repo)/blob/\(.sha)/\(.path)#L\(.line)"
+              )
+            }) | group_by(.) | map(.[0] * { count: length }) | sort[0:100][]
+            | "<tr>"
+            +   "<td><strong>\(.count)</strong></td>"
+            +   "<td><a href=\"\(.source_url)\">\(.klass)#\(.NAME)</a></td>"
+            +   "<td><pre>\(.failure)</pre></td>"
+            + "</tr>"
           ' reports/*/report.json >>$GITHUB_STEP_SUMMARY
 
           cat <<EOF >>$GITHUB_STEP_SUMMARY
           </tbody>
           </table>
           EOF
 
+          # Do not print json if too large.
+          [[ "$(du -s reports | cut -f1)" -gt 124 ]] && exit 0
+
           echo >>$GITHUB_STEP_SUMMARY
           echo '```json' >>$GITHUB_STEP_SUMMARY
           jq --slurp --compact-output '.' reports/*/report.json >>$GITHUB_STEP_SUMMARY

.github/workflows/flaky.yml

@@ -44,6 +44,7 @@ jobs:
       ruby: ${{ steps.generate-matrix.outputs.ruby }}
       seeds: ${{ steps.generate-matrix.outputs.seeds }}
   test:
+    timeout-minutes: 9
     runs-on: ubuntu-latest
     needs: prepare-matrix
     strategy:
@@ -72,7 +73,7 @@ jobs:
           name: report-${{ matrix.crdb }}-${{ matrix.ruby }}-${{ matrix.seed }}
           path: report.json
   collect:
-    if: failure()
+    if: failure() || cancelled()
     needs: test
     runs-on: ubuntu-latest
     steps:
@@ -97,16 +98,20 @@ jobs:
           EOF
 
           jq --slurp --raw-output '
-            sort_by(.total_time)[]
-            | {seed, time: .total_time | strftime("%H:%M:%S"), failure: .failed_tests[0].NAME }
-            | "<tr><td>\(.seed)</td><td>\(.time)</td><td>\(.failure)</td></tr>"
+            sort_by(.total_time)[0:100][]
+            | {seed, time: .total_time | strftime("%H:%M:%S"), klass: .failed_tests[0].klass, test: .failed_tests[0].NAME }
+            | "<tr><td>\(.seed)</td><td>\(.time)</td><td>\(.klass)#\(.test)</td></tr>"
           ' reports/*/report.json >> $GITHUB_STEP_SUMMARY
 
 
           cat <<EOF >> $GITHUB_STEP_SUMMARY
           </tbody>
           </table>
           EOF
+
+          # Do not print json if too large.
+          [[ "$(du -s reports | cut -f1)" -gt 124 ]] && exit 0
+
           echo >> $GITHUB_STEP_SUMMARY
           echo '```json' >> $GITHUB_STEP_SUMMARY
           jq --slurp --compact-output '.' reports/*/report.json >> $GITHUB_STEP_SUMMARY

Gemfile

@@ -52,6 +52,7 @@ group :development, :test do
   gem "msgpack", ">= 1.7.0"
   gem "mutex_m", "~> 0.2.0"
 
+  gem "tracer"
   gem "rake"
   gem "debug"
   gem "minitest-bisect", github: "BuonOmo/minitest-bisect", branch: "main"

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", "~> 8.0.0"
+  spec.add_dependency "activerecord", "~> 8.1.0"
   spec.add_dependency "pg", "~> 1.5"
-  spec.add_dependency "rgeo-activerecord", "~> 8.0.0"
+  spec.add_dependency "rgeo-activerecord", "~> 8.1.0"
 
   spec.add_development_dependency "benchmark-ips", "~> 2.9.1"
 
@@ -29,10 +29,8 @@ Gem::Specification.new do |spec|
       "public gem pushes."
   end
 
-  spec.files         = `git ls-files -z`.split("\x0").reject do |f|
-    f.match(%r{^(test|spec|features)/})
-  end
-  spec.bindir        = "exe"
-  spec.executables   = spec.files.grep(%r{^exe/}) { |f| File.basename(f) }
-  spec.require_paths = ["lib"]
+  git_files = `git ls-files -z`.split("\x0")
+  spec.files = (Dir["lib/**/*.rb"] + %w(LICENSE Rakefile Gemfile)) & git_files
+  spec.extra_rdoc_files = Dir["**/*.md"] & git_files
+  spec.test_files = Dir["test/**/*"] & git_files
 end

bin/start-cockroachdb

@@ -19,7 +19,7 @@ fi
 
 cockroach start-single-node \
 	--insecure --store=type=mem,size=0.25 --advertise-addr=localhost \
-	--spatial-libs="$(geos-config --includes)" \
+	--spatial-libs="$(geos-config --prefix)/lib" \
 	--pid-file "$pid_file" \
 	&> "$log_file" &
 

lib/active_record/connection_adapters/cockroachdb/column.rb

@@ -20,33 +20,29 @@ 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
-          @geographic = !!(sql_type_metadata.sql_type =~ /geography\(/i)
+                       serial: nil, generated: nil, hidden: nil)
+
+          super(name, cast_type, default, sql_type_metadata, null, default_function,
+            collation: collation, comment: comment, serial: serial, generated: generated, identity: identity)
+
+          @geographic = sql_type_metadata.sql_type.match?(/geography\(/i)
           @hidden = hidden
 
-          if spatial
-            # This case comes from an entry in the geometry_columns table
-            set_geometric_type_from_name(spatial[:type])
-            @srid = spatial[:srid].to_i
-            @has_z = !!spatial[:has_z]
-            @has_m = !!spatial[:has_m]
-          elsif @geographic
+          if @geographic
             # Geographic type information is embedded in the SQL type
             @srid = 4326
             @has_z = @has_m = false
             build_from_sql_type(sql_type_metadata.sql_type)
-          elsif sql_type =~ /geography|geometry|point|linestring|polygon/i
+          elsif sql_type.match?(/geography|geometry|point|linestring|polygon/i)
             build_from_sql_type(sql_type_metadata.sql_type)
-          elsif sql_type_metadata.sql_type =~ /geography|geometry|point|linestring|polygon/i
+          elsif sql_type_metadata.sql_type.match?(/geography|geometry|point|linestring|polygon/i)
             # A geometry column with no geometry_columns entry.
             # @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,
-            collation: collation, comment: comment, serial: serial, generated: generated, identity: identity)
+
           if spatial? && @srid
             @limit = { srid: @srid, type: to_type_name(geometric_type) }
             @limit[:has_z] = true if @has_z
@@ -59,20 +55,18 @@ def initialize(name, default, sql_type_metadata = nil, null = true,
                     :geometric_type,
                     :has_m,
                     :has_z,
-                    :srid
+                    :srid,
+                    :hidden
 
         alias geographic? geographic
         alias has_z? has_z
         alias has_m? has_m
+        alias hidden? hidden
 
         def limit
           spatial? ? @limit : super
         end
 
-        def hidden?
-          @hidden
-        end
-
         def spatial?
           %i[geometry geography].include?(@sql_type_metadata.type)
         end
@@ -81,15 +75,63 @@ def serial?
           default_function == 'unique_rowid()'
         end
 
-        private
+        # TODO: add tests (see #390)
+        def init_with(coder)
+          @geographic = coder["geographic"]
+          @geometric_type = coder["geometric_type"]
+          @has_m = coder["has_m"]
+          @has_z = coder["has_z"]
+          @srid = coder["srid"]
+          @hidden = coder["hidden"]
+          @limit = coder["limit"]
+          super
+        end
+
+        # TODO: add tests (see #390)
+        def encode_with(coder)
+          coder["geographic"] = @geographic
+          coder["geometric_type"] = @geometric_type
+          coder["has_m"] = @has_m
+          coder["has_z"] = @has_z
+          coder["srid"] = @srid
+          coder["hidden"] = @hidden
+          coder["limit"] = @limit
+          super
+        end
+
+        # TODO: add tests (see #390)
+        def ==(other)
+          other.is_a?(Column) &&
+            super &&
+            other.geographic == geographic &&
+            other.geometric_type == geometric_type &&
+            other.has_m == has_m &&
+            other.has_z == has_z &&
+            other.srid == srid &&
+            other.hidden == hidden &&
+            other.limit == limit
 
-        def set_geometric_type_from_name(name)
-          @geometric_type = RGeo::ActiveRecord.geometric_type_from_name(name) || RGeo::Feature::Geometry
         end
+        alias :eql? :==
+
+        # TODO: add tests (see #390)
+        def hash
+          Column.hash ^
+            super.hash ^
+            geographic.hash ^
+            geometric_type.hash ^
+            has_m.hash ^
+            has_z.hash ^
+            srid.hash ^
+            hidden.hash ^
+            limit.hash
+        end
+
+        private
 
         def build_from_sql_type(sql_type)
           geo_type, @srid, @has_z, @has_m = OID::Spatial.parse_sql_type(sql_type)
-          set_geometric_type_from_name(geo_type)
+          @geometric_type = RGeo::ActiveRecord.geometric_type_from_name(geo_type) || RGeo::Feature::Geometry
         end
 
         def to_type_name(geometric_type)

lib/active_record/connection_adapters/cockroachdb/database_statements.rb

@@ -34,7 +34,6 @@ def insert_fixtures_set(fixture_set, tables_to_delete = [])
           # our statements by calling `#execute` instead of `#execute_batch`.
           #
           # [1]: https://github.com/rails/rails/pull/52428
-
           begin # much faster without disabling referential integrity, worth trying.
             transaction(requires_new: true) do
               execute(statements, "Fixtures Load")

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

@@ -26,9 +26,22 @@ class Spatial < Type::Value
           #   "geometry(Polygon,4326) NOT NULL"
           #   "geometry(Geography,4326)"
           def initialize(oid, sql_type)
-            @sql_type = sql_type
-            @geo_type, @srid, @has_z, @has_m = self.class.parse_sql_type(sql_type)
+            super()
+            @sql_type = sql_type.freeze
+            @factory_attrs = self.class
+              .parse_sql_type(sql_type)
+              .then { |geo_type, srid, has_z, has_m|
+                {
+                  geo_type: geo_type.underscore.freeze,
+                  srid: srid.freeze,
+                  has_z: has_z.freeze,
+                  has_m: has_m.freeze,
+                  sql_type: type.to_s.freeze
+                }
+              }
+              .freeze
           end
+          protected attr_reader :sql_type, :factory_attrs
 
           # sql_type: geometry, geometry(Point), geometry(Point,4326), ...
           #
@@ -59,15 +72,8 @@ 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/
+            @sql_type.start_with?("geography")
           end
 
           def spatial?
@@ -92,6 +98,19 @@ def serialize(value)
                                      .generate(geo_value)
           end
 
+          # TODO: add tests (see #390)
+          def ==(other)
+            super &&
+              @sql_type == other.sql_type &&
+              @factory_attrs == other.factory_attrs
+          end
+          alias eql? ==
+
+          # TODO: add tests (see #390)
+          def hash
+            super ^ [@sql_type, @factory_attrs].hash
+          end
+
           private
 
           def cast_value(value)
@@ -108,7 +127,7 @@ 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)
@@ -119,14 +138,10 @@ def wkt_parser(string)
             end
           end
 
-          def factory_attrs
-            {
-              geo_type: @geo_type.underscore,
-              has_m: @has_m,
-              has_z: @has_z,
-              srid: @srid,
-              sql_type: type.to_s
-            }
+          def spatial_factory
+            RGeo::ActiveRecord::SpatialFactoryStore.instance.factory(
+              factory_attrs
+            )
           end
         end
       end