cockroachdb · BuonOmo · Oct 2, 2024
diff --git a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb
@@ -77,21 +77,15 @@ 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
               AND attr.attnum       = dep.refobjsubid
               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 attr.attishidden  = false
               AND cons.contype      = 'p'
               AND dep.classid       = 'pg_class'::regclass
               AND dep.refobjid      = #{quote(quote_table_name(table))}::regclass

diff --git a/lib/active_record/connection_adapters/cockroachdb_adapter.rb b/lib/active_record/connection_adapters/cockroachdb_adapter.rb
@@ -414,11 +414,8 @@ def column_definitions(table_name)
           fields.delete_if do |field|
             # Don't include rowid column if it is hidden and the primary key
             # is not defined (meaning CRDB implicitly created it).
-            if field[0] == CockroachDBAdapter::DEFAULT_PRIMARY_KEY
+            field[0] == CockroachDBAdapter::DEFAULT_PRIMARY_KEY &&
               field[10] && !primary_key(table_name)
-            else
-              false # Keep this entry.
-            end
           end
         end
 

diff --git a/test/cases/helper_cockroachdb.rb b/test/cases/helper_cockroachdb.rb
@@ -26,7 +26,7 @@ def current_adapter?(...)
 module LoadSchemaHelperExt
   def load_schema
     # TODO: Remove this const_set mess once https://github.com/rails/rails/commit/d5c2ff8345c9d23b7326edb2bbe72b6e86a63140
-    #   is part of a rails release.
+    #   is part of a rails release (likely 8.0.0).
     old_helper = ActiveRecord::TestCase
     ActiveRecord.const_set(:TestCase, NoPGSchemaTestCase.new(ActiveRecord::TestCase))
     return if ENV['COCKROACH_LOAD_FROM_TEMPLATE']

diff --git a/test/cases/migration/columns_test.rb b/test/cases/migration/columns_test.rb
diff --git a/test/excludes/ActiveRecord/AdapterTest.rb b/test/excludes/ActiveRecord/AdapterTest.rb
@@ -1,3 +1,5 @@
-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."
+transactional_test_msg = "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_indexes, transactional_test_msg
+exclude :test_remove_index_when_name_and_wrong_column_name_specified, transactional_test_msg
+exclude :test_remove_index_when_name_and_wrong_column_name_specified_positional_argument, transactional_test_msg
diff --git a/test/excludes/ActiveRecord/Migration/ColumnsTest.rb b/test/excludes/ActiveRecord/Migration/ColumnsTest.rb
diff --git a/test/excludes/AttributeMethodsTest.rb b/test/excludes/AttributeMethodsTest.rb
diff --git a/test/excludes/BulkAlterTableMigrationsTest.rb b/test/excludes/BulkAlterTableMigrationsTest.rb
@@ -1,7 +1,8 @@
 exclude :test_changing_columns, "Type conversion from DATE to TIMESTAMP requires overwriting existing values which is not yet implemented. https://github.com/cockroachdb/cockroach/issues/9851"
 exclude :test_changing_column_null_with_default, "Type conversion from DATE to TIMESTAMP requires overwriting existing values which is not yet implemented. https://github.com/cockroachdb/cockroach/issues/9851"
 
-exclude :test_adding_indexes, "Need to reference the specific query count for CockroachDB"
-exclude :test_removing_index, "Need to reference the specific query count for CockroachDB"
-exclude :test_adding_multiple_columns, "Need to reference the specific query count for CockroachDB"
-exclude :test_changing_index, "Need to reference the specific query count for CockroachDB"
+query_count_msg = "Need to reference the specific query count for CockroachDB. Fixed in test/cases/migration_test.rb"
+exclude :test_adding_indexes, query_count_msg
+exclude :test_removing_index, query_count_msg
+exclude :test_adding_multiple_columns, query_count_msg
+exclude :test_changing_index, query_count_msg
diff --git a/test/excludes/PostgresqlNetworkTest.rb b/test/excludes/PostgresqlNetworkTest.rb
@@ -36,7 +36,7 @@ def on_send(node)
     case node
     in [:send, [:const, nil, :PostgresqlNetworkAddress], /create|new/, [:hash, *pairs]]
 
-      pairs.each_with_index do |pair, i|
+      pairs.each do |pair|
         if pair in [:pair, [:sym, /(cidr|mac)_address/], *]
           expr = pair.location.expression
           large_expr = expr.resize(expr.size + 1)