From 9040b6130b59b53d11ad6cfa28d1e2204baa875c Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 9 Oct 2025 13:18:37 +0200 Subject: [PATCH 01/21] feat: upgrade to Rails 8.1 --- activerecord-cockroachdb-adapter.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord-cockroachdb-adapter.gemspec b/activerecord-cockroachdb-adapter.gemspec index 9492cb0e..542d058b 100644 --- a/activerecord-cockroachdb-adapter.gemspec +++ b/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" From 6fae89ccbeb5c1a02734a88b3ebe2f1848ef1e91 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 16 Oct 2025 18:15:35 +0200 Subject: [PATCH 02/21] fix: add `cast_type` to `Column.new` --- lib/active_record/connection_adapters/cockroachdb/column.rb | 4 ++-- .../connection_adapters/cockroachdb/schema_statements.rb | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/active_record/connection_adapters/cockroachdb/column.rb b/lib/active_record/connection_adapters/cockroachdb/column.rb index fdaab7c7..eee40652 100644 --- a/lib/active_record/connection_adapters/cockroachdb/column.rb +++ b/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) } diff --git a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb index ba5f469a..a7500781 100644 --- a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb +++ b/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, From 8891c5e5db0604cca0a412a28f39955f369c2b98 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Fri, 17 Oct 2025 13:25:04 +0200 Subject: [PATCH 03/21] fix(test): don't alter backtrace from gems --- .../AssociationDeprecationTest/NotifyModeTest.rb | 2 ++ .../RaiseBacktraceModeTest.rb | 2 ++ .../AssociationDeprecationTest/RaiseModeTest.rb | 2 ++ .../WarnBacktraceModeTest.rb | 2 ++ .../AssociationDeprecationTest/WarnModeTest.rb | 2 ++ .../fix_backtrace_cleaner.rb | 10 ++++++++++ 6 files changed, 20 insertions(+) create mode 100644 test/excludes/AssociationDeprecationTest/NotifyModeTest.rb create mode 100644 test/excludes/AssociationDeprecationTest/RaiseBacktraceModeTest.rb create mode 100644 test/excludes/AssociationDeprecationTest/RaiseModeTest.rb create mode 100644 test/excludes/AssociationDeprecationTest/WarnBacktraceModeTest.rb create mode 100644 test/excludes/AssociationDeprecationTest/WarnModeTest.rb create mode 100644 test/excludes/AssociationDeprecationTest/fix_backtrace_cleaner.rb diff --git a/test/excludes/AssociationDeprecationTest/NotifyModeTest.rb b/test/excludes/AssociationDeprecationTest/NotifyModeTest.rb new file mode 100644 index 00000000..ed1f6040 --- /dev/null +++ b/test/excludes/AssociationDeprecationTest/NotifyModeTest.rb @@ -0,0 +1,2 @@ +require_relative "fix_backtrace_cleaner" +include(FixBacktraceCleaner) diff --git a/test/excludes/AssociationDeprecationTest/RaiseBacktraceModeTest.rb b/test/excludes/AssociationDeprecationTest/RaiseBacktraceModeTest.rb new file mode 100644 index 00000000..ed1f6040 --- /dev/null +++ b/test/excludes/AssociationDeprecationTest/RaiseBacktraceModeTest.rb @@ -0,0 +1,2 @@ +require_relative "fix_backtrace_cleaner" +include(FixBacktraceCleaner) diff --git a/test/excludes/AssociationDeprecationTest/RaiseModeTest.rb b/test/excludes/AssociationDeprecationTest/RaiseModeTest.rb new file mode 100644 index 00000000..ed1f6040 --- /dev/null +++ b/test/excludes/AssociationDeprecationTest/RaiseModeTest.rb @@ -0,0 +1,2 @@ +require_relative "fix_backtrace_cleaner" +include(FixBacktraceCleaner) diff --git a/test/excludes/AssociationDeprecationTest/WarnBacktraceModeTest.rb b/test/excludes/AssociationDeprecationTest/WarnBacktraceModeTest.rb new file mode 100644 index 00000000..ed1f6040 --- /dev/null +++ b/test/excludes/AssociationDeprecationTest/WarnBacktraceModeTest.rb @@ -0,0 +1,2 @@ +require_relative "fix_backtrace_cleaner" +include(FixBacktraceCleaner) diff --git a/test/excludes/AssociationDeprecationTest/WarnModeTest.rb b/test/excludes/AssociationDeprecationTest/WarnModeTest.rb new file mode 100644 index 00000000..ed1f6040 --- /dev/null +++ b/test/excludes/AssociationDeprecationTest/WarnModeTest.rb @@ -0,0 +1,2 @@ +require_relative "fix_backtrace_cleaner" +include(FixBacktraceCleaner) diff --git a/test/excludes/AssociationDeprecationTest/fix_backtrace_cleaner.rb b/test/excludes/AssociationDeprecationTest/fix_backtrace_cleaner.rb new file mode 100644 index 00000000..9ed08029 --- /dev/null +++ b/test/excludes/AssociationDeprecationTest/fix_backtrace_cleaner.rb @@ -0,0 +1,10 @@ +module FixBacktraceCleaner + def setup + super + bc = ActiveSupport::BacktraceCleaner.new + bc.remove_silencers! + bc.remove_filters! + bc.add_silencer { !_1.include?(::AssociationDeprecationTest::TestCase::THIS_FILE) } + ActiveRecord::LogSubscriber.backtrace_cleaner = bc + end +end From 62b6ea1dc38dddc61590c2ab1cfb991a166ddd46 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Fri, 17 Oct 2025 15:09:03 +0200 Subject: [PATCH 04/21] fix(oid): freeze spatial oid Rails is moving towards ractor compatibility, and to do so it swapped to frozen database objects. Hence we cannot simply memoize the factories anymore. I've checked the initialization speed for factories, and the memoization was too fast for it to be worth implementing a cache. See https://github.com/rails/rails/commit/d2da81670c0049aeb2b7b062a80dd069ee2e725a See https://github.com/rails/rails/commit/76448a01667f9d477e6884a986276687167dfc83 --- .../cockroachdb/oid/spatial.rb | 17 +++++++---------- .../connection_adapters/cockroachdb_adapter.rb | 2 +- test/cases/adapters/postgresql/postgis_test.rb | 8 +------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/lib/active_record/connection_adapters/cockroachdb/oid/spatial.rb b/lib/active_record/connection_adapters/cockroachdb/oid/spatial.rb index eed4ffc7..740b8421 100644 --- a/lib/active_record/connection_adapters/cockroachdb/oid/spatial.rb +++ b/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 diff --git a/lib/active_record/connection_adapters/cockroachdb_adapter.rb b/lib/active_record/connection_adapters/cockroachdb_adapter.rb index 3a98cbff..489ba726 100644 --- a/lib/active_record/connection_adapters/cockroachdb_adapter.rb +++ b/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 diff --git a/test/cases/adapters/postgresql/postgis_test.rb b/test/cases/adapters/postgresql/postgis_test.rb index f3dd811d..561cd10a 100644 --- a/test/cases/adapters/postgresql/postgis_test.rb +++ b/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 From 3c90a6dc9f3e66979d8463b2b1e7f012729afbfe Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Fri, 17 Oct 2025 15:55:24 +0200 Subject: [PATCH 05/21] fix: update run_command execution --- .../connection_adapters/cockroachdb/database_tasks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_record/connection_adapters/cockroachdb/database_tasks.rb b/lib/active_record/connection_adapters/cockroachdb/database_tasks.rb index b951e4f6..c5abc93e 100644 --- a/lib/active_record/connection_adapters/cockroachdb/database_tasks.rb +++ b/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 From 98b6baedfb042cfd0be87747b68f95f3b2c172af Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 23 Oct 2025 15:09:26 +0200 Subject: [PATCH 06/21] fix: bump rgeo-activerecord --- activerecord-cockroachdb-adapter.gemspec | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord-cockroachdb-adapter.gemspec b/activerecord-cockroachdb-adapter.gemspec index 542d058b..4d61ca68 100644 --- a/activerecord-cockroachdb-adapter.gemspec +++ b/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.1.0.a" + 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" From 355924189835ee6ac9092cb08e7b52055c5e6d36 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 23 Oct 2025 16:58:12 +0200 Subject: [PATCH 07/21] fix: use class level native_database_type --- .../cockroachdb/schema_statements.rb | 17 ----------------- .../connection_adapters/cockroachdb_adapter.rb | 17 +++++++++++++++++ 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb index a7500781..17aa9425 100644 --- a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb +++ b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb @@ -296,23 +296,6 @@ def type_to_sql(type, limit: nil, precision: nil, scale: nil, array: nil, **) # sql end - # override - def native_database_types - # Add spatial types - super.merge( - geography: { name: "geography" }, - geometry: { name: "geometry" }, - geometry_collection: { name: "geometry_collection" }, - line_string: { name: "line_string" }, - multi_line_string: { name: "multi_line_string" }, - multi_point: { name: "multi_point" }, - multi_polygon: { name: "multi_polygon" }, - spatial: { name: "geometry" }, - st_point: { name: "st_point" }, - st_polygon: { name: "st_polygon" } - ) - end - # override def create_table_definition(*args, **kwargs) CockroachDB::TableDefinition.new(self, *args, **kwargs) diff --git a/lib/active_record/connection_adapters/cockroachdb_adapter.rb b/lib/active_record/connection_adapters/cockroachdb_adapter.rb index 489ba726..009b0675 100644 --- a/lib/active_record/connection_adapters/cockroachdb_adapter.rb +++ b/lib/active_record/connection_adapters/cockroachdb_adapter.rb @@ -111,6 +111,23 @@ def self.spatial_column_options(key) SPATIAL_COLUMN_OPTIONS[key] end + def self.native_database_types + return @native_database_types if defined?(@native_database_types) + # Add spatial types + @native_database_types = super.merge( + geography: { name: "geography" }, + geometry: { name: "geometry" }, + geometry_collection: { name: "geometry_collection" }, + line_string: { name: "line_string" }, + multi_line_string: { name: "multi_line_string" }, + multi_point: { name: "multi_point" }, + multi_polygon: { name: "multi_polygon" }, + spatial: { name: "geometry" }, + st_point: { name: "st_point" }, + st_polygon: { name: "st_polygon" } + ) + end + def postgis_lib_version @postgis_lib_version ||= select_value("SELECT PostGIS_Lib_Version()") end From 0cf5c832d61693eca25d15d9aa2d54c0985c3593 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Fri, 24 Oct 2025 14:28:22 +0200 Subject: [PATCH 08/21] fix(test): default is now serialized See https://github.com/rails/rails/commit/205cdd50ba626325b26ce2919d5e961c6df50df0 --- test/cases/defaults_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cases/defaults_test.rb b/test/cases/defaults_test.rb index ca6cadc2..5fbdd995 100644 --- a/test/cases/defaults_test.rb +++ b/test/cases/defaults_test.rb @@ -39,7 +39,7 @@ class DefaultNumber < ActiveRecord::Base; end def test_default_decimal_zero_with_large_scale record = DefaultNumber.new assert_equal 0.0, record.decimal_number - assert_equal "0.0000000000000000", record.decimal_number_before_type_cast + assert_equal 0.0, record.decimal_number_before_type_cast end end end From 27e9e593b8780ce3bf9231200548541bd656dd77 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 30 Oct 2025 11:12:37 +0100 Subject: [PATCH 09/21] fix(schema): hidden columns - update some outdated overrides. - update override comments to know when was the last override. - remove the `#column_names_from_column_numbers` method that was generating N+1 queries. - update related methods. See: - https://github.com/rails/rails/commit/249c367b4d3a65a3c0406c4a508cb2ba0493eed9 - https://github.com/rails/rails/commit/4e0546c5e6c2aef4bb1db771492be0bdc1202eab - https://github.com/rails/rails/commit/c93d1b09fcc013033af506b10fd60829267be85c --- .../cockroachdb/referential_integrity.rb | 48 ++-- .../cockroachdb/schema_statements.rb | 246 +++++++++++++----- .../cockroachdb_adapter.rb | 21 +- 3 files changed, 216 insertions(+), 99 deletions(-) diff --git a/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb b/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb index fa4cb179..a8b138ad 100644 --- a/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb +++ b/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb @@ -85,8 +85,8 @@ def disable_referential_integrity private - # Copy/paste of the `#foreign_keys(table)` method adapted to return every single - # foreign key in the database. + # NOTE: Copy/paste of the `#foreign_keys(table)` method adapted + # to return every single foreign key in the database. def all_foreign_keys fk_info = internal_exec_query(<<~SQL, "SCHEMA") SELECT CASE @@ -99,14 +99,30 @@ def all_foreign_keys THEN '' ELSE n2.nspname || '.' END || t2.relname AS to_table, - a1.attname AS column, a2.attname AS primary_key, c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete, c.convalidated AS valid, c.condeferrable AS deferrable, c.condeferred AS deferred, - c.conkey, c.confkey, c.conrelid, c.confrelid + c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete, c.convalidated AS valid, c.condeferrable AS deferrable, c.condeferred AS deferred, c.conrelid, c.confrelid, + ( + SELECT array_agg(a.attname ORDER BY idx) + FROM ( + SELECT idx, c.conkey[idx] AS conkey_elem + FROM generate_subscripts(c.conkey, 1) AS idx + ) indexed_conkeys + JOIN pg_attribute a ON a.attrelid = t1.oid + AND a.attnum = indexed_conkeys.conkey_elem + AND NOT a.attishidden + ) AS conkey_names, + ( + SELECT array_agg(a.attname ORDER BY idx) + FROM ( + SELECT idx, c.confkey[idx] AS confkey_elem + FROM generate_subscripts(c.confkey, 1) AS idx + ) indexed_confkeys + JOIN pg_attribute a ON a.attrelid = t2.oid + AND a.attnum = indexed_confkeys.confkey_elem + AND NOT a.attishidden + ) AS confkey_names FROM pg_constraint c JOIN pg_class t1 ON c.conrelid = t1.oid JOIN pg_class t2 ON c.confrelid = t2.oid - JOIN pg_attribute a1 ON a1.attnum = c.conkey[1] AND a1.attrelid = t1.oid - JOIN pg_attribute a2 ON a2.attnum = c.confkey[1] AND a2.attrelid = t2.oid - JOIN pg_namespace t3 ON c.connamespace = t3.oid JOIN pg_namespace n1 ON t1.relnamespace = n1.oid JOIN pg_namespace n2 ON t2.relnamespace = n2.oid WHERE c.contype = 'f' @@ -116,22 +132,16 @@ def all_foreign_keys fk_info.map do |row| from_table = PostgreSQL::Utils.unquote_identifier(row["from_table"]) to_table = PostgreSQL::Utils.unquote_identifier(row["to_table"]) - conkey = row["conkey"].scan(/\d+/).map(&:to_i) - confkey = row["confkey"].scan(/\d+/).map(&:to_i) - - if conkey.size > 1 - column = column_names_from_column_numbers(row["conrelid"], conkey) - primary_key = column_names_from_column_numbers(row["confrelid"], confkey) - else - column = PostgreSQL::Utils.unquote_identifier(row["column"]) - primary_key = row["primary_key"] - end + + column = decode_string_array(row["conkey_names"]) + primary_key = decode_string_array(row["confkey_names"]) options = { - column: column, + column: column.size == 1 ? column.first : column, name: row["name"], - primary_key: primary_key + primary_key: primary_key.size == 1 ? primary_key.first : primary_key } + options[:on_delete] = extract_foreign_key_action(row["on_delete"]) options[:on_update] = extract_foreign_key_action(row["on_update"]) options[:deferrable] = extract_constraint_deferrable(row["deferrable"], row["deferred"]) diff --git a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb index 17aa9425..d850ec04 100644 --- a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb +++ b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb @@ -20,6 +20,91 @@ module CockroachDB module SchemaStatements include ActiveRecord::ConnectionAdapters::PostgreSQL::SchemaStatements + # OVERRIDE(v8.1.1): + # - prepend Utils with PostgreSQL:: + # - handle hidden attributes + # Returns an array of indexes for the given table. + def indexes(table_name) # :nodoc: + scope = quoted_scope(table_name) + + result = query(<<~SQL, "SCHEMA") + SELECT distinct i.relname, d.indisunique, d.indkey, pg_get_indexdef(d.indexrelid), + pg_catalog.obj_description(i.oid, 'pg_class') AS comment, d.indisvalid, + ARRAY( + SELECT pg_get_indexdef(d.indexrelid, k + 1, true) + FROM generate_subscripts(d.indkey, 1) AS k + ORDER BY k + ) AS columns, + ARRAY( + SELECT a.attname + FROM pg_attribute a + WHERE a.attrelid = d.indexrelid AND a.attishidden + ) AS hidden_columns + FROM pg_class t + INNER JOIN pg_index d ON t.oid = d.indrelid + INNER JOIN pg_class i ON d.indexrelid = i.oid + LEFT JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE i.relkind IN ('i', 'I') + AND d.indisprimary = 'f' + AND t.relname = #{scope[:name]} + AND n.nspname = #{scope[:schema]} + ORDER BY i.relname + SQL + + unquote = -> (column) { + PostgreSQL::Utils.unquote_identifier(column.strip.gsub('""', '"')) + } + result.map do |row| + index_name = row[0] + unique = row[1] + indkey = row[2].split(" ").map(&:to_i) + inddef = row[3] + comment = row[4] + valid = row[5] + columns = decode_string_array(row[6]).map(&unquote) + hidden_columns = decode_string_array(row[7]).map(&unquote) + + using, expressions, include, nulls_not_distinct, where = inddef.scan(/ USING (\w+?) \((.+?)\)(?: INCLUDE \((.+?)\))?( NULLS NOT DISTINCT)?(?: WHERE (.+))?\z/m).flatten + + orders = {} + opclasses = {} + include_columns = include ? include.split(",").map(&unquote) : [] + + if indkey.include?(0) + columns = expressions + else + # prevent INCLUDE and hidden columns from being matched + columns.reject! { |c| include_columns.include?(c) || hidden_columns.include?(c) } + + # add info on sort order (only desc order is explicitly specified, asc is the default) + # and non-default opclasses + expressions.scan(/(?\w+)"?\s?(?\w+_ops(_\w+)?)?\s?(?DESC)?\s?(?NULLS (?:FIRST|LAST))?/).each do |column, opclass, desc, nulls| + opclasses[column] = opclass.to_sym if opclass + if nulls + orders[column] = [desc, nulls].compact.join(" ") + else + orders[column] = :desc if desc + end + end + end + + IndexDefinition.new( + table_name, + index_name, + unique, + columns, + orders: orders, + opclasses: opclasses, + where: where, + using: using.to_sym, + include: include_columns.presence, + nulls_not_distinct: nulls_not_distinct.present?, + comment: comment.presence, + valid: valid + ) + end + end + # OVERRIDE: We do not want to see the crdb_internal schema in the names. # # Returns an array of schema names. @@ -55,34 +140,18 @@ def primary_key(table_name) end end + # OVERRIDE(v8.1.1): handle hidden attributes def primary_keys(table_name) - return super unless database_version >= 24_02_02 - query_values(<<~SQL, "SCHEMA") SELECT a.attname - FROM ( - SELECT indrelid, indkey, generate_subscripts(indkey, 1) idx - FROM pg_index - WHERE indrelid = #{quote(quote_table_name(table_name))}::regclass - AND indisprimary - ) i - JOIN pg_attribute a - ON a.attrelid = i.indrelid - AND a.attnum = i.indkey[i.idx] - AND NOT a.attishidden - ORDER BY i.idx - SQL - end - - def column_names_from_column_numbers(table_oid, column_numbers) - return super unless database_version >= 24_02_02 - - Hash[query(<<~SQL, "SCHEMA")].values_at(*column_numbers).compact - SELECT a.attnum, a.attname - FROM pg_attribute a - WHERE a.attrelid = #{table_oid} - AND a.attnum IN (#{column_numbers.join(", ")}) - AND NOT a.attishidden + FROM pg_index i + JOIN pg_attribute a + ON a.attrelid = i.indrelid + AND a.attnum = ANY(i.indkey) + AND NOT a.attishidden + WHERE i.indrelid = #{quote(quote_table_name(table_name))}::regclass + AND i.indisprimary + ORDER BY array_position(i.indkey, a.attnum) SQL end @@ -94,7 +163,7 @@ def foreign_key_options(from_table, to_table, options) options end - # OVERRIDE: Added `unique_rowid` to the last line of the second query. + # OVERRIDE(v8.1.1): Added `unique_rowid` to the last line of the second query. # This is a CockroachDB-specific function used for primary keys. # Also make sure we don't consider `NOT VISIBLE` columns. # @@ -108,11 +177,7 @@ def pk_and_sequence_for(table) # :nodoc: pg_attribute attr, pg_depend dep, pg_constraint cons, - pg_namespace nsp, - -- TODO: use the pg_catalog.pg_attribute(attishidden) column when - -- it is added instead of joining on crdb_internal. - -- See https://github.com/cockroachdb/cockroach/pull/126397 - crdb_internal.table_columns tc + pg_namespace nsp WHERE seq.oid = dep.objid AND seq.relkind = 'S' AND attr.attrelid = dep.refobjid @@ -120,12 +185,10 @@ def pk_and_sequence_for(table) # :nodoc: AND attr.attrelid = cons.conrelid AND attr.attnum = cons.conkey[1] AND seq.relnamespace = nsp.oid - AND attr.attrelid = tc.descriptor_id - AND attr.attname = tc.column_name - AND tc.hidden = false AND cons.contype = 'p' AND dep.classid = 'pg_class'::regclass AND dep.refobjid = #{quote(quote_table_name(table))}::regclass + AND not attr.attishidden SQL if result.nil? || result.empty? @@ -143,12 +206,8 @@ def pk_and_sequence_for(table) # :nodoc: JOIN pg_attrdef def ON (adrelid = attrelid AND adnum = attnum) JOIN pg_constraint cons ON (conrelid = adrelid AND adnum = conkey[1]) JOIN pg_namespace nsp ON (t.relnamespace = nsp.oid) - -- TODO: use the pg_catalog.pg_attribute(attishidden) column when - -- it is added instead of joining on crdb_internal. - -- See https://github.com/cockroachdb/cockroach/pull/126397 - JOIN crdb_internal.table_columns tc ON (attr.attrelid = tc.descriptor_id AND attr.attname = tc.column_name) WHERE t.oid = #{quote(quote_table_name(table))}::regclass - AND tc.hidden = false + AND NOT attr.attishidden AND cons.contype = 'p' AND pg_get_expr(def.adbin, def.adrelid) ~* 'nextval|uuid_generate|gen_random_uuid|unique_rowid' SQL @@ -164,53 +223,66 @@ def pk_and_sequence_for(table) # :nodoc: nil end - # override - # Modified version of the postgresql foreign_keys method. - # Replaces t2.oid::regclass::text with t2.relname since this is - # more efficient in CockroachDB. - # Also, CockroachDB does not append the schema name in relname, - # so we append it manually. + # OVERRIDE(v8.1.1): + # - Replaces t2.oid::regclass::text with t2.relname + # since this is more efficient in CockroachDB. + # - prepend schema name to relname (see `AS to_table`) + # - handle hidden attributes. + # + # NOTE: If you edit this method, you'll need to edit + # the `#all_foreign_keys` method as well. def foreign_keys(table_name) scope = quoted_scope(table_name) - fk_info = internal_exec_query(<<~SQL, "SCHEMA") + fk_info = internal_exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false) SELECT CASE WHEN n2.nspname = current_schema() THEN '' ELSE n2.nspname || '.' END || t2.relname AS to_table, - a1.attname AS column, a2.attname AS primary_key, c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete, c.convalidated AS valid, c.condeferrable AS deferrable, c.condeferred AS deferred, - c.conkey, c.confkey, c.conrelid, c.confrelid + c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete, c.convalidated AS valid, c.condeferrable AS deferrable, c.condeferred AS deferred, c.conrelid, c.confrelid, + ( + SELECT array_agg(a.attname ORDER BY idx) + FROM ( + SELECT idx, c.conkey[idx] AS conkey_elem + FROM generate_subscripts(c.conkey, 1) AS idx + ) indexed_conkeys + JOIN pg_attribute a ON a.attrelid = t1.oid + AND a.attnum = indexed_conkeys.conkey_elem + AND NOT a.attishidden + ) AS conkey_names, + ( + SELECT array_agg(a.attname ORDER BY idx) + FROM ( + SELECT idx, c.confkey[idx] AS confkey_elem + FROM generate_subscripts(c.confkey, 1) AS idx + ) indexed_confkeys + JOIN pg_attribute a ON a.attrelid = t2.oid + AND a.attnum = indexed_confkeys.confkey_elem + AND NOT a.attishidden + ) AS confkey_names FROM pg_constraint c JOIN pg_class t1 ON c.conrelid = t1.oid JOIN pg_class t2 ON c.confrelid = t2.oid - JOIN pg_attribute a1 ON a1.attnum = c.conkey[1] AND a1.attrelid = t1.oid - JOIN pg_attribute a2 ON a2.attnum = c.confkey[1] AND a2.attrelid = t2.oid - JOIN pg_namespace t3 ON c.connamespace = t3.oid + JOIN pg_namespace n1 ON t1.relnamespace = n1.oid JOIN pg_namespace n2 ON t2.relnamespace = n2.oid WHERE c.contype = 'f' AND t1.relname = #{scope[:name]} - AND t3.nspname = #{scope[:schema]} + AND n1.nspname = #{scope[:schema]} ORDER BY c.conname SQL fk_info.map do |row| to_table = PostgreSQL::Utils.unquote_identifier(row["to_table"]) - conkey = row["conkey"].scan(/\d+/).map(&:to_i) - confkey = row["confkey"].scan(/\d+/).map(&:to_i) - if conkey.size > 1 - column = column_names_from_column_numbers(row["conrelid"], conkey) - primary_key = column_names_from_column_numbers(row["confrelid"], confkey) - else - column = PostgreSQL::Utils.unquote_identifier(row["column"]) - primary_key = row["primary_key"] - end + column = decode_string_array(row["conkey_names"]) + primary_key = decode_string_array(row["confkey_names"]) options = { - column: column, + column: column.size == 1 ? column.first : column, name: row["name"], - primary_key: primary_key + primary_key: primary_key.size == 1 ? primary_key.first : primary_key } + options[:on_delete] = extract_foreign_key_action(row["on_delete"]) options[:on_update] = extract_foreign_key_action(row["on_update"]) options[:deferrable] = extract_constraint_deferrable(row["deferrable"], row["deferred"]) @@ -228,8 +300,52 @@ def default_sequence_name(table_name, pk = "id") nil end - # override - # https://github.com/rails/rails/blob/6-0-stable/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb#L624 + # OVERRIDE(v8.1.1): handle hidden attributes + # + # Returns an array of unique constraints for the given table. + # The unique constraints are represented as UniqueConstraintDefinition objects. + def unique_constraints(table_name) + scope = quoted_scope(table_name) + + unique_info = internal_exec_query(<<~SQL, "SCHEMA", allow_retry: true, materialize_transactions: false) + SELECT c.conname, c.conrelid, c.condeferrable, c.condeferred, pg_get_constraintdef(c.oid) AS constraintdef, + ( + SELECT array_agg(a.attname ORDER BY idx) + FROM ( + SELECT idx, c.conkey[idx] AS conkey_elem + FROM generate_subscripts(c.conkey, 1) AS idx + ) indexed_conkeys + JOIN pg_attribute a ON a.attrelid = t.oid + AND a.attnum = indexed_conkeys.conkey_elem + AND NOT a.attishidden + ) AS conkey_names + FROM pg_constraint c + JOIN pg_class t ON c.conrelid = t.oid + JOIN pg_namespace n ON n.oid = c.connamespace + WHERE c.contype = 'u' + AND t.relname = #{scope[:name]} + AND n.nspname = #{scope[:schema]} + SQL + + unique_info.map do |row| + columns = decode_string_array(row["conkey_names"]) + + nulls_not_distinct = row["constraintdef"].start_with?("UNIQUE NULLS NOT DISTINCT") + deferrable = extract_constraint_deferrable(row["condeferrable"], row["condeferred"]) + + options = { + name: row["conname"], + nulls_not_distinct: nulls_not_distinct, + deferrable: deferrable + } + + UniqueConstraintDefinition.new(table_name, columns, options) + end + end + + # OVERRIDE(v8.1.1): + # - Add spatial information + # - Add hidden information def new_column_from_field(table_name, field, _definition) column_name, type, default, notnull, oid, fmod, collation, comment, identity, attgenerated, hidden = field type_metadata = fetch_type_metadata(column_name, type, oid.to_i, fmod.to_i) diff --git a/lib/active_record/connection_adapters/cockroachdb_adapter.rb b/lib/active_record/connection_adapters/cockroachdb_adapter.rb index 009b0675..a3c70b84 100644 --- a/lib/active_record/connection_adapters/cockroachdb_adapter.rb +++ b/lib/active_record/connection_adapters/cockroachdb_adapter.rb @@ -395,18 +395,10 @@ def extract_decimal_from_default(default) nil end - # override - # This method makes a query to gather information about columns - # in a table. It returns an array of arrays (one for each col) and - # passes each to the SchemaStatements#new_column_from_field method - # as the field parameter. This data is then used to format the column - # objects for the model and sent to the OID for data casting. - # - # Sometimes there are differences between how data is formatted - # in Postgres and CockroachDB, so additional queries for certain types - # may be necessary to properly form the column definition. - # - # @see: https://github.com/rails/rails/blob/8695b028261bdd244e254993255c6641bdbc17a5/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L829 + # OVERRIDE(v8.1.1): + # - comment is retrieved differently than PG for performance + # - gather detailed information about spatial columns. See + # `#crdb_column_definitions` def column_definitions(table_name) fields = query(<<~SQL, "SCHEMA") SELECT a.attname, format_type(a.atttypid, a.atttypmod), @@ -414,7 +406,7 @@ def column_definitions(table_name) c.collname, NULL AS comment, attidentity, attgenerated, - NULL as is_hidden + a.attishidden FROM pg_attribute a LEFT JOIN pg_attrdef d ON a.attrelid = d.adrelid AND a.attnum = d.adnum LEFT JOIN pg_type t ON a.atttypid = t.oid @@ -445,7 +437,6 @@ def column_definitions(table_name) dtype = field[f_type] field[f_type] = crdb_fields[field[f_attname]][2].downcase if re.match(dtype) field[f_comment] = crdb_fields[field[f_attname]][1] - field[f_is_hidden] = true if crdb_fields[field[f_attname]][3] field end fields.delete_if do |field| @@ -467,7 +458,7 @@ def crdb_column_definitions(table_name) with_schema = " AND c.table_schema = #{quote(table_name.schema)}" if table_name.schema fields = \ query(<<~SQL, "SCHEMA") - SELECT c.column_name, c.column_comment, c.crdb_sql_type, c.is_hidden::BOOLEAN + SELECT c.column_name, c.column_comment, c.crdb_sql_type FROM information_schema.columns c WHERE c.table_name = #{quote(table)}#{with_schema} SQL From 6cc929dc0160da6b0b38f5d91dbb146be0142d65 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 30 Oct 2025 13:44:56 +0100 Subject: [PATCH 10/21] fix(ci): smaller summary without skips --- .github/workflows/ci.yml | 5 ++++- .github/workflows/flaky.yml | 8 ++++++-- .../connection_adapters/cockroachdb/quoting.rb | 7 ++++--- test/cases/helper_cockroachdb.rb | 2 +- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f81df9c6..49158499 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,7 +58,7 @@ jobs: EOF jq --slurp --raw-output ' - map(.failed_tests|map("\(.klass)#\(.NAME)")) | flatten | unique | sort | to_entries[] + map(.failed_tests|map("\(.klass)#\(.NAME)")) | flatten | unique | sort[0:100] | to_entries[] | "\(.key)\(.value)" ' reports/*/report.json >>$GITHUB_STEP_SUMMARY @@ -67,6 +67,9 @@ jobs: 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 diff --git a/.github/workflows/flaky.yml b/.github/workflows/flaky.yml index fdcd0e06..1c9e6f9c 100644 --- a/.github/workflows/flaky.yml +++ b/.github/workflows/flaky.yml @@ -72,7 +72,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,7 +97,7 @@ jobs: EOF jq --slurp --raw-output ' - sort_by(.total_time)[] + sort_by(.total_time)[0:100][] | {seed, time: .total_time | strftime("%H:%M:%S"), failure: .failed_tests[0].NAME } | "\(.seed)\(.time)\(.failure)" ' reports/*/report.json >> $GITHUB_STEP_SUMMARY @@ -107,6 +107,10 @@ jobs: 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 diff --git a/lib/active_record/connection_adapters/cockroachdb/quoting.rb b/lib/active_record/connection_adapters/cockroachdb/quoting.rb index 44decd30..cd1f86ec 100644 --- a/lib/active_record/connection_adapters/cockroachdb/quoting.rb +++ b/lib/active_record/connection_adapters/cockroachdb/quoting.rb @@ -34,7 +34,8 @@ module Quoting # but when creating objects, using RGeo features is more convenient than # converting to WKB, so this does it automatically. def quote(value) - if value.is_a?(Numeric) + case value + when Numeric # NOTE: The fact that integers are quoted is important and helps # mitigate a potential vulnerability. # @@ -42,9 +43,9 @@ def quote(value) # - https://nvd.nist.gov/vuln/detail/CVE-2022-44566 # - https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/280#discussion_r1288692977 "'#{quote_string(value.to_s)}'" - elsif RGeo::Feature::Geometry.check_type(value) + when RGeo::Feature::Geometry "'#{RGeo::WKRep::WKBGenerator.new(hex_format: true, type_format: :ewkb, emit_ewkb_srid: true).generate(value)}'" - elsif value.is_a?(RGeo::Cartesian::BoundingBox) + when RGeo::Cartesian::BoundingBox "'#{value.min_x},#{value.min_y},#{value.max_x},#{value.max_y}'::box" else super diff --git a/test/cases/helper_cockroachdb.rb b/test/cases/helper_cockroachdb.rb index 6ff746e3..b2415b53 100644 --- a/test/cases/helper_cockroachdb.rb +++ b/test/cases/helper_cockroachdb.rb @@ -180,7 +180,7 @@ def report seed: Minitest.seed, assertions: assertions, count: count, - failed_tests: results, + failed_tests: results.reject(&:skipped?), total_time: total_time, failures: failures, errors: errors, From ba0cfb0fe207b22e60a1453fcc46f11e7744073e Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 30 Oct 2025 16:04:44 +0100 Subject: [PATCH 11/21] fix(test): us english names --- .../ConnectionAdapters/PostgreSQLAdapterTest.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapterTest.rb b/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapterTest.rb index b45bcd60..d63b7d17 100644 --- a/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapterTest.rb +++ b/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapterTest.rb @@ -24,11 +24,11 @@ # NOTE: The expression `do $$ BEGIN RAISE WARNING 'foo'; END; $$` works with PG, not CRDB. plpgsql_needed = "PL-PGSQL differs in CockroachDB. Not tested yet. See #339" -exclude :test_ignores_warnings_when_behaviour_ignore, plpgsql_needed -exclude :test_logs_warnings_when_behaviour_log, plpgsql_needed -exclude :test_raises_warnings_when_behaviour_raise, plpgsql_needed -exclude :test_reports_when_behaviour_report, plpgsql_needed -exclude :test_warnings_behaviour_can_be_customized_with_a_proc, plpgsql_needed +exclude :test_ignores_warnings_when_behavior_ignore, plpgsql_needed +exclude :test_logs_warnings_when_behavior_log, plpgsql_needed +exclude :test_raises_warnings_when_behavior_raise, plpgsql_needed +exclude :test_reports_when_behavior_report, plpgsql_needed +exclude :test_warnings_behavior_can_be_customized_with_a_proc, plpgsql_needed exclude :test_allowlist_of_warnings_to_ignore, plpgsql_needed exclude :test_allowlist_of_warning_codes_to_ignore, plpgsql_needed From 5f7b588f1ffdfd789b4a60dc80a30992f85298c7 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Thu, 30 Oct 2025 16:05:17 +0100 Subject: [PATCH 12/21] fix(test): coerced value for generate column --- test/excludes/PostgresqlVirtualColumnTest.rb | 15 +++++++++++++++ test/support/copy_cat.rb | 3 +++ 2 files changed, 18 insertions(+) diff --git a/test/excludes/PostgresqlVirtualColumnTest.rb b/test/excludes/PostgresqlVirtualColumnTest.rb index 059cbf5e..5c2f7bc7 100644 --- a/test/excludes/PostgresqlVirtualColumnTest.rb +++ b/test/excludes/PostgresqlVirtualColumnTest.rb @@ -1,2 +1,17 @@ +require "support/copy_cat" + exclude :test_build_fixture_sql, "Skipping because CockroachDB cannot write directly to computed columns." exclude :test_schema_dumping, "Replaced with local version" + +# CRDB doesn't support implicit casts. +# See https://github.com/cockroachdb/cockroach/issues/75101 +# +# From: "ASCII(name)" +# To: "ASCII(name)""::string" +CopyCat.copy_methods(self, self, :test_change_table_without_stored_option) do + def on_str(node) + return unless node in [:str, "ASCII(name)"] + + insert_after(node.loc.expression, '"::string"') + end +end diff --git a/test/support/copy_cat.rb b/test/support/copy_cat.rb index 30803fc5..4c4ec79a 100644 --- a/test/support/copy_cat.rb +++ b/test/support/copy_cat.rb @@ -21,6 +21,9 @@ def warn(message, category: nil, **kwargs) # Copy methods from The original rails class to our adapter. # While copying, we can rewrite the source code of the method using # ast. Use `debug: true` to lead you through that process. + # + # Once debug is set, you can check the closest node you want to edit + # and then create a method `on_` to handle it. def copy_methods(new_klass, old_klass, *methods, debug: false, &block) methods.each do |met| file, _ = old_klass.instance_method(met).source_location From 471d131f6280bc57ee7dd8a4f6f8015235926ef1 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 3 Nov 2025 14:19:20 +0100 Subject: [PATCH 13/21] fix(referential-integrity): make sure fks are added back This would silently fail in the test `test_bulk_insert_with_a_multi_statement_query_raises_an_exception_when_any_insert_fails`. Removing some foreign keys and thus failing afterwards in the test suite. Moreover, the `#disable_referential_integrity` method is unfortunately public, so we need it to be robust. --- .../cockroachdb/referential_integrity.rb | 72 ++++++++++--------- 1 file changed, 39 insertions(+), 33 deletions(-) diff --git a/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb b/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb index a8b138ad..85abc43a 100644 --- a/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb +++ b/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb @@ -36,6 +36,29 @@ def check_all_foreign_keys_valid! def disable_referential_integrity foreign_keys = all_foreign_keys + remove_foreign_keys(foreign_keys) + + # Prefixes and suffixes are added in add_foreign_key + # in AR7+ so we need to temporarily disable them here, + # otherwise prefixes/suffixes will be erroneously added. + old_prefix = ActiveRecord::Base.table_name_prefix + old_suffix = ActiveRecord::Base.table_name_suffix + + begin + yield + ensure + ActiveRecord::Base.table_name_prefix = "" + ActiveRecord::Base.table_name_suffix = "" + add_foreign_keys(foreign_keys) # Never raises. + + ActiveRecord::Base.table_name_prefix = old_prefix if defined?(old_prefix) + ActiveRecord::Base.table_name_suffix = old_suffix if defined?(old_suffix) + end + end + + private + + def remove_foreign_keys(foreign_keys) statements = foreign_keys.map do |foreign_key| # We do not use the `#remove_foreign_key` method here because it # checks for foreign keys existance in the schema cache. This method @@ -46,45 +69,28 @@ def disable_referential_integrity schema_creation.accept(at) end execute_batch(statements, "Disable referential integrity -> remove foreign keys") + end - yield - - # Prefixes and suffixes are added in add_foreign_key - # in AR7+ so we need to temporarily disable them here, - # otherwise prefixes/suffixes will be erroneously added. - old_prefix = ActiveRecord::Base.table_name_prefix - old_suffix = ActiveRecord::Base.table_name_suffix + # NOTE: This method should never raise, otherwise we risk polluting table name + # prefixes and suffixes. The good thing is: if this happens, tests will crash + # hard, no way we miss it. + def add_foreign_keys(foreign_keys) + # We avoid using `foreign_key_exists?` here because it checks the schema cache + # for every key. This method is performance critical for the test suite, hence + # we use the `#all_foreign_keys` method that only make one query to the database. + already_inserted_foreign_keys = all_foreign_keys + statements = foreign_keys.map do |foreign_key| + next if already_inserted_foreign_keys.any? { |fk| fk.from_table == foreign_key.from_table && fk.options[:name] == foreign_key.options[:name] } - ActiveRecord::Base.table_name_prefix = "" - ActiveRecord::Base.table_name_suffix = "" + options = foreign_key_options(foreign_key.from_table, foreign_key.to_table, foreign_key.options) + at = create_alter_table foreign_key.from_table + at.add_foreign_key foreign_key.to_table, options - begin - # Avoid having PG:DuplicateObject error if a test is ran in transaction. - # TODO: verify that there is no cache issue related to running this (e.g: fk - # still in cache but not in db) - # - # We avoid using `foreign_key_exists?` here because it checks the schema cache - # for every key. This method is performance critical for the test suite, hence - # we use the `#all_foreign_keys` method that only make one query to the database. - already_inserted_foreign_keys = all_foreign_keys - statements = foreign_keys.map do |foreign_key| - next if already_inserted_foreign_keys.any? { |fk| fk.from_table == foreign_key.from_table && fk.options[:name] == foreign_key.options[:name] } - - options = foreign_key_options(foreign_key.from_table, foreign_key.to_table, foreign_key.options) - at = create_alter_table foreign_key.from_table - at.add_foreign_key foreign_key.to_table, options - - schema_creation.accept(at) - end - execute_batch(statements.compact, "Disable referential integrity -> add foreign keys") - ensure - ActiveRecord::Base.table_name_prefix = old_prefix - ActiveRecord::Base.table_name_suffix = old_suffix + schema_creation.accept(at) end + execute_batch(statements.compact, "Disable referential integrity -> add foreign keys") end - private - # NOTE: Copy/paste of the `#foreign_keys(table)` method adapted # to return every single foreign key in the database. def all_foreign_keys From d135b2535195736cefc9f4ba48bd6642512acab2 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 3 Nov 2025 15:36:51 +0100 Subject: [PATCH 14/21] refactor: cleanup old code --- Gemfile | 1 + test/cases/adapter_test.rb | 121 ------------------ .../ActiveRecord/AdapterForeignKeyTest.rb | 3 - 3 files changed, 1 insertion(+), 124 deletions(-) delete mode 100644 test/excludes/ActiveRecord/AdapterForeignKeyTest.rb diff --git a/Gemfile b/Gemfile index 971be35a..a93d777c 100644 --- a/Gemfile +++ b/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" diff --git a/test/cases/adapter_test.rb b/test/cases/adapter_test.rb index 99535ed1..866830e8 100644 --- a/test/cases/adapter_test.rb +++ b/test/cases/adapter_test.rb @@ -64,88 +64,9 @@ def test_remove_index_when_name_and_wrong_column_name_specified_positional_argum end - class AdapterForeignKeyTest < ActiveRecord::TestCase - self.use_transactional_tests = false - - fixtures :fk_test_has_pk - - def before_setup - super - conn = ActiveRecord::Base.lease_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 - super - @connection = ActiveRecord::Base.lease_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 @@ -156,7 +77,6 @@ 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 @@ -174,46 +94,5 @@ 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 diff --git a/test/excludes/ActiveRecord/AdapterForeignKeyTest.rb b/test/excludes/ActiveRecord/AdapterForeignKeyTest.rb deleted file mode 100644 index 9e82adef..00000000 --- a/test/excludes/ActiveRecord/AdapterForeignKeyTest.rb +++ /dev/null @@ -1,3 +0,0 @@ -exclude :test_foreign_key_violations_are_translated_to_specific_exception_with_validate_false, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" -exclude :test_foreign_key_violations_on_insert_are_translated_to_specific_exception, "This is override to prevent an intermittent error. Table fk_test_has_pk has constrain droped and not created back" -exclude :test_foreign_key_violations_on_delete_are_translated_to_specific_exception, "This is override to prevent an intermittent error. Table fk_test_has_pk has constrain droped and not created back" From 2defaa73ca71da5a130934a2e0b6be6ff92901ec Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Tue, 4 Nov 2025 08:34:46 +0100 Subject: [PATCH 15/21] refactor: remove `debugging?` utility --- .../cockroachdb/schema_statements.rb | 10 ---------- .../connection_adapters/cockroachdb_adapter.rb | 4 ---- test/cases/helper_cockroachdb.rb | 3 --- 3 files changed, 17 deletions(-) diff --git a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb index d850ec04..7561a95b 100644 --- a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb +++ b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb @@ -112,16 +112,6 @@ def schema_names super - ["crdb_internal"] end - def add_index(table_name, column_name, **options) - super - rescue ActiveRecord::StatementInvalid => error - if debugging? && error.cause.class == PG::FeatureNotSupported - warn "#{error}\n\nThis error will be ignored and the index will not be created.\n\n" - else - raise error - end - end - # ActiveRecord allows for tables to exist without primary keys. # Databases like PostgreSQL support this behavior, but CockroachDB does # not. If a table is created without a primary key, CockroachDB will add diff --git a/lib/active_record/connection_adapters/cockroachdb_adapter.rb b/lib/active_record/connection_adapters/cockroachdb_adapter.rb index a3c70b84..1f0caa23 100644 --- a/lib/active_record/connection_adapters/cockroachdb_adapter.rb +++ b/lib/active_record/connection_adapters/cockroachdb_adapter.rb @@ -145,10 +145,6 @@ def srs_database_columns } end - def debugging? - !!ENV["DEBUG_COCKROACHDB_ADAPTER"] - end - def max_transaction_retries @max_transaction_retries ||= @config.fetch(:max_transaction_retries, 3) end diff --git a/test/cases/helper_cockroachdb.rb b/test/cases/helper_cockroachdb.rb index b2415b53..93ab1cf6 100644 --- a/test/cases/helper_cockroachdb.rb +++ b/test/cases/helper_cockroachdb.rb @@ -12,9 +12,6 @@ module ExcludeMessage # some rails specific messages are then ignored. Minitest::Test.make_my_diffs_pretty! if ENV['VERBOSE'] -# Turn on debugging for the test environment -ENV['DEBUG_COCKROACHDB_ADAPTER'] = "1" - # Override the load_schema_helper for the # two ENV variables COCKROACH_LOAD_FROM_TEMPLATE # and COCKROACH_SKIP_LOAD_SCHEMA that can From c6aa880bfa0d801e7f478f61ca590d0baa78b39e Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Tue, 4 Nov 2025 08:56:33 +0100 Subject: [PATCH 16/21] refactor: use variables for autocommit_before_ddl This is the default intended approach, as it will quote value if needed. --- test/config.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/config.yml b/test/config.yml index f554f02c..6ef37aa0 100644 --- a/test/config.yml +++ b/test/config.yml @@ -12,7 +12,10 @@ connections: # # This options keyword is referenced here in libpq: # https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-OPTIONS - options: "-c autocommit_before_ddl=false" + # + # NOTE: with command lines or a URI, one could use `-c autocommit_before_ddl=false` + variables: + autocommit_before_ddl: false arunit_without_prepared_statements: <<: *arunit prepared_statements: false From 226f1ed4d478488244d6cb4366cbbf00de29a713 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Tue, 4 Nov 2025 10:13:40 +0100 Subject: [PATCH 17/21] feat(referential-integrity): handle autocommit_before_ddl If `#disable_referential_integrity` is ran within a transaction, we may be able to still run the user's code. Otherwise we warn the user why it failed. --- .../cockroachdb/database_statements.rb | 1 - .../cockroachdb/referential_integrity.rb | 54 ++++++++++++------- .../cockroachdb/referential_integrity_test.rb | 51 ++++++++++++++++++ .../PostgreSQLReferentialIntegrityTest.rb | 18 +++++-- 4 files changed, 100 insertions(+), 24 deletions(-) create mode 100644 test/cases/adapters/cockroachdb/referential_integrity_test.rb diff --git a/lib/active_record/connection_adapters/cockroachdb/database_statements.rb b/lib/active_record/connection_adapters/cockroachdb/database_statements.rb index 100b4ca0..9a2a2312 100644 --- a/lib/active_record/connection_adapters/cockroachdb/database_statements.rb +++ b/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") diff --git a/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb b/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb index 85abc43a..e8b818ef 100644 --- a/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb +++ b/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb @@ -34,25 +34,41 @@ def check_all_foreign_keys_valid! end def disable_referential_integrity - foreign_keys = all_foreign_keys - - remove_foreign_keys(foreign_keys) - - # Prefixes and suffixes are added in add_foreign_key - # in AR7+ so we need to temporarily disable them here, - # otherwise prefixes/suffixes will be erroneously added. - old_prefix = ActiveRecord::Base.table_name_prefix - old_suffix = ActiveRecord::Base.table_name_suffix - - begin - yield - ensure - ActiveRecord::Base.table_name_prefix = "" - ActiveRecord::Base.table_name_suffix = "" - add_foreign_keys(foreign_keys) # Never raises. - - ActiveRecord::Base.table_name_prefix = old_prefix if defined?(old_prefix) - ActiveRecord::Base.table_name_suffix = old_suffix if defined?(old_suffix) + if transaction_open? && query_value("SHOW autocommit_before_ddl") == "off" + begin + yield + rescue ActiveRecord::InvalidForeignKey => e + warn <<-WARNING +WARNING: Rails was not able to disable referential integrity. + +This is due to CockroachDB's need of committing transactions +before a schema change occurs. To bypass this, you can set +`autocommit_before_ddl: "on"` in your database configuration. +WARNING + raise e + end + else + foreign_keys = all_foreign_keys + + remove_foreign_keys(foreign_keys) + + # Prefixes and suffixes are added in add_foreign_key + # in AR7+ so we need to temporarily disable them here, + # otherwise prefixes/suffixes will be erroneously added. + old_prefix = ActiveRecord::Base.table_name_prefix + old_suffix = ActiveRecord::Base.table_name_suffix + + begin + yield + ensure + ActiveRecord::Base.table_name_prefix = "" + ActiveRecord::Base.table_name_suffix = "" + + add_foreign_keys(foreign_keys) # Never raises. + + ActiveRecord::Base.table_name_prefix = old_prefix if defined?(old_prefix) + ActiveRecord::Base.table_name_suffix = old_suffix if defined?(old_suffix) + end end end diff --git a/test/cases/adapters/cockroachdb/referential_integrity_test.rb b/test/cases/adapters/cockroachdb/referential_integrity_test.rb new file mode 100644 index 00000000..2b01f7e9 --- /dev/null +++ b/test/cases/adapters/cockroachdb/referential_integrity_test.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require "cases/helper_cockroachdb" +require "support/connection_helper" # for #reset_connection +require "support/copy_cat" + +class CockroachDBReferentialIntegrityTest < ActiveRecord::PostgreSQLTestCase + include ConnectionHelper + + module ProgrammerMistake + def execute_batch(sql, name = nil) + raise ArgumentError, "something is not right." if name.match?(/referential integrity/) + super + end + end + + def setup + @connection = ActiveRecord::Base.lease_connection + end + + def teardown + reset_connection + end + + exclude_from_transactional_tests :test_only_catch_active_record_errors_others_bubble_up + CopyCat.copy_methods(self, ::PostgreSQLReferentialIntegrityTest, :test_only_catch_active_record_errors_others_bubble_up) + + def test_should_reraise_invalid_foreign_key_exception_and_show_warning + warning = capture(:stderr) do + e = assert_raises(ActiveRecord::InvalidForeignKey) do + @connection.disable_referential_integrity do + @connection.execute("INSERT INTO authors (name, author_address_id) VALUES ('Mona Chollet', 42)") + end + end + assert_match (/Key \(author_address_id\)=\(42\) is not present in table/), e.message + end + assert_match (/WARNING: Rails was not able to disable referential integrity/), warning + assert_match (/autocommit_before_ddl/), warning + end + + def test_no_warning_nor_error_with_autocommit_before_ddl + @connection.execute("SET SESSION autocommit_before_ddl = 'on'") + warning = capture(:stderr) do + @connection.disable_referential_integrity do + @connection.execute("INSERT INTO authors (name, author_address_id) VALUES ('Mona Chollet', 42)") + @connection.truncate(:authors) + end + end + assert_predicate warning, :blank?, "expected no warnings but got:\n#{warning}" + end +end diff --git a/test/excludes/PostgreSQLReferentialIntegrityTest.rb b/test/excludes/PostgreSQLReferentialIntegrityTest.rb index b3c7bcb7..9565eb39 100644 --- a/test/excludes/PostgreSQLReferentialIntegrityTest.rb +++ b/test/excludes/PostgreSQLReferentialIntegrityTest.rb @@ -1,4 +1,14 @@ -exclude :test_only_catch_active_record_errors_others_bubble_up, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" -exclude :test_should_reraise_invalid_foreign_key_exception_and_show_warning, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" -exclude :test_does_not_break_transactions, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" -exclude :test_does_not_break_nested_transactions, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" +exclude :test_should_reraise_invalid_foreign_key_exception_and_show_warning, + "CockroachDB has a different limitation as there is no" \ + "'DISABLE TRIGGER' statement." + +break_tx = "CockroachDB will always alter transactions when " \ + "trying to disable referential integrity. Either it cannot " \ + "work within transaction, or autocommit_before_ddl is set " \ + "and transactions will be committed." +exclude :test_does_not_break_transactions, break_tx +exclude :test_does_not_break_nested_transactions, break_tx + +exclude :test_only_catch_active_record_errors_others_bubble_up, + "Reimplemented in test/cases/adapters/cockroachdb/referential_integrity_test.rb" \ + " to use a different trigger for the error." From 2dbca8644a1ba17eb38254a37f84ab645130b085 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Tue, 4 Nov 2025 11:13:41 +0100 Subject: [PATCH 18/21] refactor: various - fix geos-config file fetching - refactor for loop into any? - unexclude working tests --- bin/start-cockroachdb | 2 +- test/cases/helper_cockroachdb.rb | 8 ++------ .../ActiveRecord/AdapterTestWithoutTransaction.rb | 2 -- test/excludes/ActiveRecord/PostgresqlConnectionTest.rb | 1 - 4 files changed, 3 insertions(+), 10 deletions(-) diff --git a/bin/start-cockroachdb b/bin/start-cockroachdb index a4268c37..39fac09f 100755 --- a/bin/start-cockroachdb +++ b/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" & diff --git a/test/cases/helper_cockroachdb.rb b/test/cases/helper_cockroachdb.rb index 93ab1cf6..30af4668 100644 --- a/test/cases/helper_cockroachdb.rb +++ b/test/cases/helper_cockroachdb.rb @@ -112,13 +112,9 @@ def run_one_method(klass, method_name, reporter) res = Minitest.run_one_method(klass, method_name) final_res ||= res - retryable = false - if res.error? - res.failures.each do |f| - retryable = true if f.message.include?("ActiveRecord::InvalidForeignKey") - end - end + retryable = res.error? && res.failures.any? { _1.message.include?("ActiveRecord::InvalidForeignKey") } (final_res = res) && break unless retryable + end # report message from first failure or from success diff --git a/test/excludes/ActiveRecord/AdapterTestWithoutTransaction.rb b/test/excludes/ActiveRecord/AdapterTestWithoutTransaction.rb index 473369c0..9abe321c 100644 --- a/test/excludes/ActiveRecord/AdapterTestWithoutTransaction.rb +++ b/test/excludes/ActiveRecord/AdapterTestWithoutTransaction.rb @@ -1,3 +1 @@ exclude :test_reset_empty_table_with_custom_pk, "The test fails because serial primary keys in CockroachDB are created with unique_rowid() where PostgreSQL will create them with a sequence. See https://www.cockroachlabs.com/docs/v19.2/serial.html#modes-of-operation" -exclude :test_truncate_tables, "This is override to prevent an intermittent error. Table fk_test_has_pk has constrain droped and not created back" -exclude :test_truncate_tables_with_query_cache, "This is override to prevent an intermittent error. Table fk_test_has_pk has constrain droped and not created back" diff --git a/test/excludes/ActiveRecord/PostgresqlConnectionTest.rb b/test/excludes/ActiveRecord/PostgresqlConnectionTest.rb index 681eaa01..371ace24 100644 --- a/test/excludes/ActiveRecord/PostgresqlConnectionTest.rb +++ b/test/excludes/ActiveRecord/PostgresqlConnectionTest.rb @@ -1,4 +1,3 @@ -exclude :test_default_client_min_messages, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_get_and_release_advisory_lock, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_reconnection_after_actual_disconnection_with_verify, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_reset, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" From e21bdaf7f1bd77bd8678ce558428134ea936f0d6 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Tue, 4 Nov 2025 13:32:27 +0100 Subject: [PATCH 19/21] feat(ci): more complete error output --- .github/workflows/ci.yml | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 49158499..a51e6d6d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,16 +50,33 @@ jobs: - + + EOF jq --slurp --raw-output ' - map(.failed_tests|map("\(.klass)#\(.NAME)")) | flatten | unique | sort[0:100] | to_entries[] - | "" + map(.failed_tests) | flatten | map({ + klass, + NAME, + failure: .failures[0], + source_url: ( + .source_location | if (.[0] | contains("/gems/")) then + (.[0] | capture("rails-(?.*?)/(?.*)")) * + {line: .[1], server: "https://github.com", repo: "rails/rails"} + else + {path: .[0], 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][] + | "" + + "" + + "" + + "" + + "" ' reports/*/report.json >>$GITHUB_STEP_SUMMARY cat <>$GITHUB_STEP_SUMMARY From 40b579ac90582352a35ecbcf26a3a86a90cd4b0a Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Tue, 4 Nov 2025 16:30:59 +0100 Subject: [PATCH 20/21] fix(test): test_remove_foreign_key_on_8_0 Since CockroachDB does not support DDL transactions, we adapt the test. --- .../Migration/CompatibilityTest.rb | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb b/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb index aecd68fd..8852fe68 100644 --- a/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb +++ b/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb @@ -17,3 +17,34 @@ def on_sym(node) insert_after(node.loc.expression, "_and_actually_way_longer_because_cockroach_is_in_the_128_game") end end + +# CockroachDB does not support DDL transactions. Hence the migration is +# not rolled back and the already removed index is not restored. +# +# From: +# if current_adapter?(:PostgreSQLAdapter, :SQLite3Adapter) +# assert_equal 2, foreign_keys.size +# else +# assert_equal 1, foreign_keys.size +# end +# To: +# assert_equal 1, foreign_keys.size +CopyCat.copy_methods(self, self, :test_remove_foreign_key_on_8_0) do + def on_if(node) + return unless node in + [:if, + [:send, nil, :current_adapter?, + [:sym, :PostgreSQLAdapter], + [:sym, :SQLite3Adapter]], + [:send, nil, :assert_equal, + [:int, 2], + [:send, + [:lvar, :foreign_keys], :size]], + [:send, nil, :assert_equal, + [:int, 1], + [:send, + [:lvar, :foreign_keys], :size]] => else_block] + + replace(node.loc.expression, else_block.location.expression.source) + end +end From d477fd60b2b5859c1741def0e5517b984494e363 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Tue, 4 Nov 2025 17:22:37 +0100 Subject: [PATCH 21/21] fix(test): type_map not ractor shareable --- test/excludes/ActiveRecord/AdapterTest.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/excludes/ActiveRecord/AdapterTest.rb b/test/excludes/ActiveRecord/AdapterTest.rb index c8c31d6e..16e1f3a4 100644 --- a/test/excludes/ActiveRecord/AdapterTest.rb +++ b/test/excludes/ActiveRecord/AdapterTest.rb @@ -1,3 +1,4 @@ exclude :test_indexes, "Rails transactional tests are being used while making schema changes. See https://www.cockroachlabs.com/docs/stable/online-schema-changes.html#limited-support-for-schema-changes-within-transactions." exclude :test_remove_index_when_name_and_wrong_column_name_specified, "Rails transactional tests are being used while making schema changes. See https://www.cockroachlabs.com/docs/stable/online-schema-changes.html#limited-support-for-schema-changes-within-transactions." exclude :test_remove_index_when_name_and_wrong_column_name_specified_positional_argument, "Rails transactional tests are being used while making schema changes. See https://www.cockroachlabs.com/docs/stable/online-schema-changes.html#limited-support-for-schema-changes-within-transactions." +exclude :test_type_map_is_ractor_shareable, "Not yet, see https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/389"
# TestFailure
\(.key)\(.value)
\(.count)\(.klass)#\(.NAME)
\(.failure)