fix: remove max_identifier_length override (#317)

This is no longer necessary as CRDB now supports
`SHOW max_identifier_length`, which is the same as
PostgreSQL's adapter implementation.

Fixes #260

Author: BuonOmo

CHANGELOG.md

@@ -2,12 +2,13 @@
 
 ## Ongoing
 
-- Add support for [AOST](cockroachlabs.com/docs/stable/as-of-system-time) queries ([#284](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/284))
-- Dump schema name in foreign keys ([#289](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/289))
+- Support arbitrary max identifier length ([#317](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/317)).
 
 ## 7.1.0 - 2024-01-30
 
 - Add support for Rails 7.1 ([#300](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/300)).
+- Add support for [AOST](cockroachlabs.com/docs/stable/as-of-system-time) queries ([#284](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/284))
+- Dump schema name in foreign keys ([#289](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/289))
 
 ## 7.0.3 - 2023-08-23
 

Gemfile

@@ -56,6 +56,9 @@ group :development, :test do
   gem "minitest-excludes", "~> 2.0.1"
   gem "minitest-github_action_reporter", github: "BuonOmo/minitest-github_action_reporter", require: "minitest/github_action_reporter_plugin"
 
+  # Gems used for tests meta-programming.
+  gem "parser"
+
   # Gems used by the ActiveRecord test suite
   gem "bcrypt", "~> 3.1.18"
   gem "mocha", "~> 1.14.0"

lib/active_record/connection_adapters/cockroachdb_adapter.rb

@@ -232,21 +232,6 @@ def supports_deferrable_constraints?
         false
       end
 
-      # This is hardcoded to 63 (as previously was in ActiveRecord 5.0) to aid in
-      # migration from PostgreSQL to CockroachDB. In practice, this limitation
-      # is arbitrary since CockroachDB supports index name lengths and table alias
-      # lengths far greater than this value. For the time being though, we match
-      # the original behavior for PostgreSQL to simplify migrations.
-      #
-      # Note that in the migration to ActiveRecord 5.1, this was changed in
-      # PostgreSQLAdapter to use `SHOW max_identifier_length` (which does not
-      # exist in CockroachDB). Therefore, we have to redefine this here.
-      def max_identifier_length
-        63
-      end
-      alias index_name_length max_identifier_length
-      alias table_alias_length max_identifier_length
-
       # NOTE: This commented bit of code allows to have access to crdb version,
       # which can be useful for feature detection. However, we currently don't
       # need, hence we avoid the extra queries.

test/cases/migration/compatibility_test.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+require "cases/helper_cockroachdb"
+require "support/copy_cat"
+
+module ActiveRecord
+  module CockroachDB
+    class Migration
+      class CompatibilityTest < ActiveRecord::Migration::CompatibilityTest
+        CopyCat.copy_methods(self, ActiveRecord::Migration::CompatibilityTest,
+          :test_add_index_errors_on_too_long_name_7_0,
+          :test_create_table_add_index_errors_on_too_long_name_7_0
+        ) do
+          def on_sym(node)
+            return unless node.children[0] == :very_long_column_name_to_test_with
+
+            insert_after(node.loc.expression, "_and_actually_way_longer_because_cockroach_is_in_the_128_game")
+          end
+        end
+      end
+    end
+  end
+end

test/excludes/ActiveRecord/Migration/CompatibilityTest.rb

@@ -6,3 +6,6 @@
 ::DefaultPrecisionImplicitTestCases.undef_method(:test_datetime_doesnt_set_precision_on_change_column)
 ::DefaultPrecisionSixTestCases.undef_method(:test_datetime_sets_precision_6_on_change_column)
 BaseCompatibilityTest.descendants.each { _1.use_transactional_tests = false }
+
+exclude :test_add_index_errors_on_too_long_name_7_0, "The max length in CRDB is 128, not 64."
+exclude :test_create_table_add_index_errors_on_too_long_name_7_0, "The max length in CRDB is 128, not 64."

test/support/copy_cat.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+require "parser/current"
+
+module CopyCat
+  extend self
+
+  # 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 true that process.
+  def copy_methods(new_klass, old_klass, *methods, debug: false, &block)
+    methods.each do |met|
+      file, _ = old_klass.instance_method(met).source_location
+      ast = find_method(Parser::CurrentRuby.parse_file(file), met)
+      code =
+        if block_given?
+          source = ast.location.expression.source
+          buffer = Parser::Source::Buffer.new(met, source: source)
+          # We need to recompute the ast to have correct locations.
+          ast = Parser::CurrentRuby.parse(source)
+
+          if debug
+            puts "=" * 80
+            puts "Rewriter doc: https://www.rubydoc.info/gems/parser/3.3.0.5/Parser/TreeRewriter"
+            puts "Pattern matching doc: https://docs.ruby-lang.org/en/master/syntax/pattern_matching_rdoc.html"
+            puts
+            puts "Method: #{met}"
+            puts
+            puts "Source:"
+            puts buffer.source
+            puts
+            puts "AST:"
+            pp ast
+            puts
+          end
+          rewriter_class = Class.new(Parser::TreeRewriter, &block)
+          rewriter_class.new.rewrite(buffer, ast)
+        else
+          ast.location.expression.source
+        end
+      if debug and block_given?
+        puts "Rewritten source:"
+        puts code
+        puts "=" * 80
+      end
+      location = caller_locations(3, 1).first
+      new_klass.class_eval(code, location.absolute_path, location.lineno)
+    end
+  end
+
+  def find_method(ast, method_name)
+    method_name = method_name.to_sym
+    to_search = [ast]
+    while !to_search.empty?
+      node = to_search.shift
+      next unless node.is_a?(Parser::AST::Node)
+      if node in [:def, ^method_name, *]
+        return node
+      end
+      to_search += node.children
+    end
+    return nil
+  end
+end