feat: support datetime precision

This is possible since v23.1.13. We also removed deadcode
in the process, and made CopyCat capable of overriding
method from the same class without raising.

Fixes #307

Author: BuonOmo

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Ongoing
 
+- Enable precision support ([#318](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/318))
 - Support arbitrary max identifier length ([#317](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/317)).
 - Fix `#schema_names` not to return `crdb_internal` ([#316](https://github.com/cockroachdb/activerecord-cockroachdb-adapter/pull/316))
 

bin/console

@@ -12,7 +12,7 @@ require "active_record"
 #       structure_load(Post.connection_db_config, "awesome-file.sql")
 require "active_record/connection_adapters/cockroachdb/database_tasks"
 
-schema_kind = ENV.fetch("SCHEMA_KIND", "default")
+schema_kind = ENV.fetch("SCHEMA_KIND", ENV.fetch("SCHEMA", "default"))
 
 system("cockroach sql --insecure --host=localhost:26257 --execute='drop database if exists ar_crdb_console'",
   exception: true)

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

@@ -18,7 +18,6 @@
 require "active_record/connection_adapters/cockroachdb/column"
 require "active_record/connection_adapters/cockroachdb/spatial_column_info"
 require "active_record/connection_adapters/cockroachdb/setup"
-require "active_record/connection_adapters/cockroachdb/oid/type_map_initializer"
 require "active_record/connection_adapters/cockroachdb/oid/spatial"
 require "active_record/connection_adapters/cockroachdb/oid/interval"
 require "active_record/connection_adapters/cockroachdb/oid/date_time"
@@ -157,18 +156,21 @@ def max_transaction_retries
         @max_transaction_retries ||= @config.fetch(:max_transaction_retries, 3)
       end
 
-      # CockroachDB 20.1 can run queries that work against PostgreSQL 10+.
-      def postgresql_version
-        100000
+      def get_database_version
+        major, minor, patch = query_value("SHOW crdb_version").match(/v(\d+).(\d+).(\d+)/)[1..].map(&:to_i)
+        major * 100 * 100 + minor * 100 + patch
       end
+      undef :postgresql_version
+      alias :cockroachdb_version :database_version
 
-      def supports_bulk_alter?
-        true
+      def supports_datetime_with_precision?
+        # https://github.com/cockroachdb/cockroach/pull/111400
+        database_version >= 23_01_13
       end
 
-      def supports_json?
-        # FIXME(joey): Add a version check.
-        true
+      def supports_nulls_not_distinct?
+        # https://github.com/cockroachdb/cockroach/issues/115836
+        false
       end
 
       def supports_ddl_transactions?
@@ -183,10 +185,6 @@ def supports_materialized_views?
         false
       end
 
-      def supports_partial_index?
-        true
-      end
-
       def supports_index_include?
         false
       end
@@ -202,14 +200,6 @@ def supports_expression_index?
         false
       end
 
-      def supports_datetime_with_precision?
-        false
-      end
-
-      def supports_comments?
-        true
-      end
-
       def supports_comments_in_create?
         false
       end

lib/active_record/connection_adapters/cockroachdb_adapter.rb

@@ -1,10 +1,7 @@
 exclude :test_legacy_change_column_with_null_executes_update, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_legacy_add_foreign_key_with_deferrable_true, "CRDB does not support DEFERRABLE constraints"
 exclude :test_disable_extension_on_7_0, "CRDB does not support enabling/disabling extensions."
-exclude :test_timestamps_sets_default_precision_on_create_table, "See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/307"
 # CRDB does not support ALTER COLUMN TYPE inside a transaction
-::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."

test/excludes/ActiveRecord/Migration/CompatibilityTest.rb

@@ -1,42 +1,21 @@
-exclude :test_precision_is_respected_on_timestamp_columns, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_quoting_non_standard_delimiters, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_attribute_for_inspect_for_array_field, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_strings_with_null_strings, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_attribute_for_inspect_for_array_field_for_large_array, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_column, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_default, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_string_quoting_rules_match_pg_behavior, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
+require "support/copy_cat"
+
+# Remove hstore from setup (both the extension and the column).
+CopyCat.copy_methods(self, self, :setup) do
+  def on_send(node)
+    if node in [:send, nil, :enable_extension!, [:str, "hstore"], *] | # enable_extension!("hstore", ...)
+               [:send, [:lvar, :t], :hstore, *] # t.hstore(...)
+      remove(node.location.expression)
+    end
+  end
+end
+
 exclude :test_change_column_default_with_array, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_encoding_arrays_of_utf8_strings, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_array_with_serialized_attributes, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_change_column_cant_make_non_array_column_to_array, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_not_compatible_with_serialize_array, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_strings_with_array_delimiters, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_assigning_non_array_value, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_insert_fixture, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_strings_with_quotes, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_contains_nils, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_datetime_with_timezone_awareness, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_select_with_integers, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_strings_with_commas, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_schema_dump_with_shorthand, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_type_cast_integers, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_rewrite_with_strings, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_select_with_strings, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_change_column_from_non_array_to_array, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_with_multi_dimensional_empty_strings, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_mutate_value_in_array, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_default_strings, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_where_by_attribute_with_array, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_with_empty_strings, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_uniqueness_validation, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_change_column_with_array, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_assigning_valid_pg_array_literal, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_multi_dimensional_with_integers, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_with_arbitrary_whitespace, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_type_cast_array, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_rewrite_with_integers, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_escaping, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_assigning_empty_string, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_multi_dimensional_with_strings, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_mutate_array, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"

test/excludes/PostgresqlArrayTest.rb

@@ -3,7 +3,6 @@
 exclude :test_schema_dump_interval_type, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_do_not_dump_foreign_keys_for_ignored_tables, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_schema_dump_includes_bigint_default, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
-exclude :test_schema_dump_keeps_large_precision_integer_columns_as_decimal, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48"
 exclude :test_schema_dump_with_timestamptz_datetime_format, "Re-implementing ourselves because we need CockroachDB specific methods."
 exclude :test_schema_dump_with_correct_timestamp_types_via_add_column_with_type_as_string, "Re-implementing ourselves because we need CockroachDB specific methods."
 exclude :test_timestamps_schema_dump_before_rails_7_with_timestamptz_setting, "Re-implementing ourselves because we need CockroachDB specific methods."

test/excludes/SchemaDumperTest.rb

@@ -3,6 +3,18 @@
 require "parser/current"
 
 module CopyCat
+  module NoWarnRewrite
+    def warn(message, category: nil, **kwargs)
+      if /method redefined; discarding old|previous definition of/.match?(message) &&
+         caller_locations.any? { _1.path["test/support/copy_cat.rb"] && _1.label["copy_methods"] }
+        # ignore
+      else
+        super
+      end
+    end
+  end
+  ::Warning.singleton_class.prepend NoWarnRewrite
+
   extend self
 
   # Copy methods from The original rails class to our adapter.
@@ -44,7 +56,7 @@ def copy_methods(new_klass, old_klass, *methods, debug: false, &block)
         puts "=" * 80
       end
       location = caller_locations(3, 1).first
-      new_klass.class_eval(code, location.absolute_path, location.lineno)
+      new_klass.class_eval(code, location.absolute_path || location.path, location.lineno)
     end
   end