diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index acd95ad1..7b3d68fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ on: # This allows a subsequently queued workflow run to interrupt previous runs. concurrency: - group: '${{ github.workflow }} @ ${{ github.ref }}' + group: "${{ github.workflow }} @ ${{ github.ref }}" cancel-in-progress: true jobs: @@ -39,7 +39,7 @@ jobs: strategy: matrix: crdb: [v23.1.11] - ruby: [ruby-head] + ruby: [head] name: Test (crdb=${{ matrix.crdb }} ruby=${{ matrix.ruby }}) steps: - name: Set Up Actions @@ -49,8 +49,8 @@ jobs: - name: Set Up Ruby uses: ruby/setup-ruby@v1 with: - ruby-version: ${{ matrix.ruby }} - bundler-cache: true + ruby-version: ${{ matrix.ruby }} + bundler-cache: true - name: Install and Start Cockroachdb run: | # Download CockroachDB @@ -95,4 +95,4 @@ jobs: SET CLUSTER SETTING sql.defaults.experimental_temporary_tables.enabled = 'true'; " - name: Test - run: bundle exec rake test + run: bundle exec rake test TESTOPTS='--profile=3' diff --git a/Gemfile b/Gemfile index 40da36cf..ab747eb1 100644 --- a/Gemfile +++ b/Gemfile @@ -48,12 +48,13 @@ group :development, :test do # a specific rails codebase. gem "rails", github: "rails/rails", tag: RailsTag.call - # Needed before rails update for ruby 3.4 - gem "mutex_m" + # Needed for the test suite + gem "msgpack", ">= 1.7.0" gem "rake" - gem "byebug" + gem "debug" 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 by the ActiveRecord test suite gem "bcrypt", "~> 3.1.18" diff --git a/activerecord-cockroachdb-adapter.gemspec b/activerecord-cockroachdb-adapter.gemspec index 2fb2c459..1014887a 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", "~> 7.0.3" + spec.add_dependency "activerecord", "~> 7.1.0" spec.add_dependency "pg", "~> 1.2" spec.add_dependency "rgeo-activerecord", "~> 7.0.0" diff --git a/lib/active_record/connection_adapters/cockroachdb/column.rb b/lib/active_record/connection_adapters/cockroachdb/column.rb index db37d875..598af10a 100644 --- a/lib/active_record/connection_adapters/cockroachdb/column.rb +++ b/lib/active_record/connection_adapters/cockroachdb/column.rb @@ -1,11 +1,11 @@ module ActiveRecord module ConnectionAdapters module CockroachDB - class Column < PostgreSQLColumn + 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, - default_function = nil, collation: nil, comment: nil, + default_function = nil, collation: nil, comment: nil, identity: nil, serial: nil, spatial: nil, generated: nil, hidden: nil) @sql_type_metadata = sql_type_metadata @geographic = !!(sql_type_metadata.sql_type =~ /geography\(/i) @@ -30,7 +30,7 @@ def initialize(name, default, sql_type_metadata = nil, null = true, build_from_sql_type(sql_type_metadata.sql_type) end super(name, default, sql_type_metadata, null, default_function, - collation: collation, comment: comment, serial: serial, generated: generated) + collation: collation, comment: comment, serial: serial, generated: generated, identity: identity) if spatial? && @srid @limit = { srid: @srid, type: to_type_name(geometric_type) } @limit[:has_z] = true if @has_z @@ -53,10 +53,6 @@ def limit spatial? ? @limit : super end - def virtual? - @generated.present? - end - def hidden? @hidden end diff --git a/lib/active_record/connection_adapters/cockroachdb/column_methods.rb b/lib/active_record/connection_adapters/cockroachdb/column_methods.rb index e613d5bb..19b1af1e 100644 --- a/lib/active_record/connection_adapters/cockroachdb/column_methods.rb +++ b/lib/active_record/connection_adapters/cockroachdb/column_methods.rb @@ -45,6 +45,14 @@ def st_point(name, options = {}) def st_polygon(name, options = {}) column(name, :st_polygon, **options) end + + private + + def valid_column_definition_options + spatial = [:srid, :has_z, :has_m, :geographic, :spatial_type] + crdb = [:hidden] + super + spatial + crdb + end end end diff --git a/lib/active_record/connection_adapters/cockroachdb/database_statements.rb b/lib/active_record/connection_adapters/cockroachdb/database_statements.rb index 84295bb8..9d3e49a3 100644 --- a/lib/active_record/connection_adapters/cockroachdb/database_statements.rb +++ b/lib/active_record/connection_adapters/cockroachdb/database_statements.rb @@ -2,21 +2,6 @@ module ActiveRecord module ConnectionAdapters module CockroachDB module DatabaseStatements - # Since CockroachDB will run all transactions with serializable isolation, - # READ UNCOMMITTED, READ COMMITTED, and REPEATABLE READ are all aliases - # for SERIALIZABLE. This lets the adapter support all isolation levels, - # but READ UNCOMMITTED has been removed from this list because the - # ActiveRecord transaction isolation test fails for READ UNCOMMITTED. - # See https://www.cockroachlabs.com/docs/v19.2/transactions.html#isolation-levels - def transaction_isolation_levels - { - read_committed: "READ COMMITTED", - repeatable_read: "REPEATABLE READ", - serializable: "SERIALIZABLE", - read_uncommitted: "READ UNCOMMITTED" - } - end - # Overridden to avoid using transactions for schema creation. def insert_fixtures_set(fixture_set, tables_to_delete = []) fixture_inserts = build_fixture_statements(fixture_set) @@ -29,74 +14,6 @@ def insert_fixtures_set(fixture_set, tables_to_delete = []) end end end - - private - def execute_batch(statements, name = nil) - statements.each do |statement| - execute(statement, name) - end - end - - DEFAULT_INSERT_VALUE = Arel.sql("DEFAULT").freeze - private_constant :DEFAULT_INSERT_VALUE - - def default_insert_value(column) - DEFAULT_INSERT_VALUE - end - - def build_fixture_sql(fixtures, table_name) - columns = schema_cache.columns_hash(table_name) - - values_list = fixtures.map do |fixture| - fixture = fixture.stringify_keys - - unknown_columns = fixture.keys - columns.keys - if unknown_columns.any? - raise Fixture::FixtureError, %(table "#{table_name}" has no columns named #{unknown_columns.map(&:inspect).join(', ')}.) - end - - columns.map do |name, column| - if fixture.key?(name) - type = lookup_cast_type_from_column(column) - with_yaml_fallback(type.serialize(fixture[name])) - else - default_insert_value(column) - end - end - end - - table = Arel::Table.new(table_name) - manager = Arel::InsertManager.new - manager.into(table) - - if values_list.size == 1 - values = values_list.shift - new_values = [] - columns.each_key.with_index { |column, i| - unless values[i].equal?(DEFAULT_INSERT_VALUE) - new_values << values[i] - manager.columns << table[column] - end - } - values_list << new_values - else - columns.each_key { |column| manager.columns << table[column] } - end - - manager.values = manager.create_values_list(values_list) - manager.to_sql - end - - def build_fixture_statements(fixture_set) - fixture_set.map do |table_name, fixtures| - next if fixtures.empty? - build_fixture_sql(fixtures, table_name) - end.compact - end - - def with_multi_statements - yield - end end end end diff --git a/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb b/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb index 09604bdf..476c21b1 100644 --- a/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb +++ b/lib/active_record/connection_adapters/cockroachdb/referential_integrity.rb @@ -15,7 +15,7 @@ module ReferentialIntegrity # referential integrity (e.g: adding a foreign key with invalid data # raises). # So foreign keys should always be valid for that matter. - def all_foreign_keys_valid? + def check_all_foreign_keys_valid! true end @@ -39,16 +39,12 @@ def disable_referential_integrity begin foreign_keys.each do |foreign_key| - begin - add_foreign_key(foreign_key.from_table, foreign_key.to_table, **foreign_key.options) - rescue ActiveRecord::StatementInvalid => error - if error.cause.class == PG::DuplicateObject - # This error is safe to ignore because the yielded caller - # already re-added the foreign key constraint. - else - raise error - end - end + # 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) + next if foreign_key_exists?(foreign_key.from_table, name: foreign_key.options[:name]) + + add_foreign_key(foreign_key.from_table, foreign_key.to_table, **foreign_key.options) end ensure ActiveRecord::Base.table_name_prefix = old_prefix diff --git a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb index 612dc9c3..77609def 100644 --- a/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb +++ b/lib/active_record/connection_adapters/cockroachdb/schema_statements.rb @@ -46,7 +46,8 @@ def foreign_keys(table_name) 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 + 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 FROM pg_constraint c JOIN pg_class t1 ON c.conrelid = t1.oid JOIN pg_class t2 ON c.confrelid = t2.oid @@ -61,15 +62,26 @@ def foreign_keys(table_name) 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 + options = { - column: PostgreSQL::Utils.unquote_identifier(row["column"]), + column: column, name: row["name"], - primary_key: row["primary_key"] + primary_key: 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_foreign_key_deferrable(row["deferrable"], row["deferred"]) + options[:deferrable] = extract_constraint_deferrable(row["deferrable"], row["deferred"]) options[:validate] = row["valid"] to_table = PostgreSQL::Utils.unquote_identifier(row["to_table"]) @@ -87,16 +99,20 @@ def default_sequence_name(table_name, pk = "id") # override # https://github.com/rails/rails/blob/6-0-stable/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb#L624 - def new_column_from_field(table_name, field) - column_name, type, default, notnull, oid, fmod, collation, comment, generated, hidden = field + 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) default_value = extract_value_from_default(default) - default_function = extract_default_function(default_value, default) - serial = - if (match = default_function&.match(/\Anextval\('"?(?.+_(?seq\d*))"?'::regclass\)\z/)) - sequence_name_from_parts(table_name, column_name, match[:suffix]) == match[:sequence_name] - end + if attgenerated.present? + default_function = default + else + default_function = extract_default_function(default_value, default) + end + + if match = default_function&.match(/\Anextval\('"?(?.+_(?seq\d*))"?'::regclass\)\z/) + serial = sequence_name_from_parts(table_name, column_name, match[:suffix]) == match[:sequence_name] + end # {:dimension=>2, :has_m=>false, :has_z=>false, :name=>"latlon", :srid=>0, :type=>"GEOMETRY"} spatial = spatial_column_info(table_name).get(column_name, type_metadata.sql_type) @@ -110,8 +126,9 @@ def new_column_from_field(table_name, field) collation: collation, comment: comment.presence, serial: serial, + identity: identity.presence, spatial: spatial, - generated: generated, + generated: attgenerated, hidden: hidden ) end diff --git a/lib/active_record/connection_adapters/cockroachdb/type.rb b/lib/active_record/connection_adapters/cockroachdb/type.rb index 77770fb9..d678fb5e 100644 --- a/lib/active_record/connection_adapters/cockroachdb/type.rb +++ b/lib/active_record/connection_adapters/cockroachdb/type.rb @@ -1,15 +1,16 @@ module ActiveRecord module Type - class << self + module CRDBExt # Return :postgresql instead of :cockroachdb for current_adapter_name so # we can continue using the ActiveRecord::Types defined in # PostgreSQLAdapter. def adapter_name_from(model) - name = model.connection_db_config.adapter.to_sym + name = super return :postgresql if name == :cockroachdb name end end + singleton_class.prepend CRDBExt end end diff --git a/lib/active_record/connection_adapters/cockroachdb_adapter.rb b/lib/active_record/connection_adapters/cockroachdb_adapter.rb index d2206255..bb1701bc 100644 --- a/lib/active_record/connection_adapters/cockroachdb_adapter.rb +++ b/lib/active_record/connection_adapters/cockroachdb_adapter.rb @@ -161,7 +161,7 @@ def postgresql_version end def supports_bulk_alter? - false + true end def supports_json? @@ -182,7 +182,15 @@ def supports_materialized_views? end def supports_partial_index? - @crdb_version >= 2020 + true + end + + def supports_index_include? + false + end + + def supports_exclusion_constraints? + false end def supports_expression_index? @@ -197,7 +205,7 @@ def supports_datetime_with_precision? end def supports_comments? - @crdb_version >= 2010 + true end def supports_comments_in_create? @@ -209,11 +217,11 @@ def supports_advisory_locks? end def supports_virtual_columns? - @crdb_version >= 2110 + true end def supports_string_to_array_coercion? - @crdb_version >= 2020 + true end def supports_partitioned_indexes? @@ -239,62 +247,30 @@ def max_identifier_length alias index_name_length max_identifier_length alias table_alias_length max_identifier_length - def initialize(connection, logger, conn_params, config) - super(connection, logger, conn_params, config) - - # crdb_version is the version of the binary running on the node. We - # really want to use `SHOW CLUSTER SETTING version` to get the cluster - # version, but that is only available to admins. Instead, we can use - # crdb_internal.is_at_least_version, but that's only available in 22.1. - crdb_version_string = query_value("SHOW crdb_version") - if crdb_version_string.include? "v22.1" - version_num = query_value(<<~SQL, "VERSION") - SELECT - CASE - WHEN crdb_internal.is_at_least_version('22.2') THEN 2220 - WHEN crdb_internal.is_at_least_version('22.1') THEN 2210 - ELSE 2120 - END; - SQL - else - # This branch can be removed once the dialect stops supporting v21.2 - # and earlier. - if crdb_version_string.include? "v1." - version_num = 1 - elsif crdb_version_string.include? "v2." - version_num 2 - elsif crdb_version_string.include? "v19.1." - version_num = 1910 - elsif crdb_version_string.include? "v19.2." - version_num = 1920 - elsif crdb_version_string.include? "v20.1." - version_num = 2010 - elsif crdb_version_string.include? "v20.2." - version_num = 2020 - elsif crdb_version_string.include? "v21.1." - version_num = 2110 - else - version_num = 2120 - end - end - @crdb_version = version_num.to_i - - # NOTE: this is normally in configure_connection, but that is run - # before crdb_version is determined. Once all supported versions - # of CockroachDB support SET intervalstyle it can safely be moved - # back. - # Set interval output format to ISO 8601 for ease of parsing by ActiveSupport::Duration.parse - if @crdb_version >= 2120 - begin - execute("SET intervalstyle_enabled = true", "SCHEMA") - execute("SET intervalstyle = iso_8601", "SCHEMA") - rescue - # Ignore any error. This can happen with a cluster that has - # not yet finalized the v21.2 upgrade. v21.2 does not have - # a way to tell if the upgrade was finalized (see comment above). - end - end - end + # 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. + # + # def initialize(connection, logger, conn_params, config) + # super(connection, logger, conn_params, config) + + # # crdb_version is the version of the binary running on the node. We + # # really want to use `SHOW CLUSTER SETTING version` to get the cluster + # # version, but that is only available to admins. Instead, we can use + # # crdb_internal.is_at_least_version, but that's only available in 22.1. + # crdb_version_string = query_value("SHOW crdb_version") + # if crdb_version_string.include? "v22.1" + # version_num = query_value(<<~SQL, "VERSION") + # SELECT + # CASE + # WHEN crdb_internal.is_at_least_version('22.2') THEN 2220 + # WHEN crdb_internal.is_at_least_version('22.1') THEN 2210 + # ELSE 2120 + # END; + # SQL + # end + # @crdb_version = version_num.to_i + # end def self.database_exists?(config) !!ActiveRecord::Base.cockroachdb_connection(config) @@ -307,12 +283,12 @@ def self.database_exists?(config) # (DO $$) that CockroachDB does not support. # # Given a name and an array of values, creates an enum type. - def create_enum(name, values) - sql_values = values.map { |s| "'#{s}'" }.join(", ") + def create_enum(name, values, **options) + sql_values = values.map { |s| quote(s) }.join(", ") query = <<~SQL - CREATE TYPE IF NOT EXISTS \"#{name}\" AS ENUM (#{sql_values}); + CREATE TYPE IF NOT EXISTS #{quote_table_name(name)} AS ENUM (#{sql_values}); SQL - exec_query(query) + internal_exec_query(query).tap { reload_type_map } end class << self @@ -370,56 +346,6 @@ def initialize_type_map(m = type_map) private - # Configures the encoding, verbosity, schema search path, and time zone of the connection. - # This is called by #connect and should not be called manually. - # - # NOTE(joey): This was cradled from postgresql_adapter.rb. This - # was due to needing to override configuration statements. - def configure_connection - if @config[:encoding] - @connection.set_client_encoding(@config[:encoding]) - end - self.client_min_messages = @config[:min_messages] || "warning" - self.schema_search_path = @config[:schema_search_path] || @config[:schema_order] - - # Use standard-conforming strings so we don't have to do the E'...' dance. - set_standard_conforming_strings - - variables = @config.fetch(:variables, {}).stringify_keys - - # If using Active Record's time zone support configure the connection to return - # TIMESTAMP WITH ZONE types in UTC. - unless variables["timezone"] - if ActiveRecord.default_timezone == :utc - variables["timezone"] = "UTC" - elsif @local_tz - variables["timezone"] = @local_tz - end - end - - # NOTE(joey): This is a workaround as CockroachDB 1.1.x - # supports SET TIME ZONE <...> and SET "time zone" = <...> but - # not SET timezone = <...>. - if variables.key?("timezone") - tz = variables.delete("timezone") - execute("SET TIME ZONE #{quote(tz)}", "SCHEMA") - end - - # SET statements from :variables config hash - # https://www.postgresql.org/docs/current/static/sql-set.html - variables.map do |k, v| - if v == ":default" || v == :default - # Sets the value to the global or compile default - - # NOTE(joey): I am not sure if simply commenting this out - # is technically correct. - # execute("SET #{k} = DEFAULT", "SCHEMA") - elsif !v.nil? - execute("SET SESSION #{k} = #{quote(v)}", "SCHEMA") - end - end - end - # Override extract_value_from_default because the upstream definition # doesn't handle the variations in CockroachDB's behavior. def extract_value_from_default(default) @@ -503,7 +429,8 @@ def column_definitions(table_name) SELECT a.attname, format_type(a.atttypid, a.atttypmod), pg_get_expr(d.adbin, d.adrelid), a.attnotnull, a.atttypid, a.atttypmod, c.collname, NULL AS comment, - #{supports_virtual_columns? ? 'attgenerated' : quote('')} as attgenerated, + attidentity, + attgenerated, NULL as is_hidden FROM pg_attribute a LEFT JOIN pg_attrdef d ON a.attrelid = d.adrelid AND a.attnum = d.adnum @@ -520,18 +447,29 @@ def column_definitions(table_name) # have [] appended to the end of it. re = /\A(?:geometry|geography|interval|numeric)/ + # 0: attname + # 1: type + # 2: default + # 3: attnotnull + # 4: atttypid + # 5: atttypmod + # 6: collname + # 7: comment + # 8: attidentity + # 9: attgenerated + # 10: is_hidden fields.map do |field| dtype = field[1] field[1] = crdb_fields[field[0]][2].downcase if re.match(dtype) field[7] = crdb_fields[field[0]][1]&.gsub!(/^\'|\'?$/, '') - field[9] = true if crdb_fields[field[0]][3] + field[10] = true if crdb_fields[field[0]][3] field end 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[9] && !primary_key(table_name) + field[10] && !primary_key(table_name) else false # Keep this entry. end @@ -592,21 +530,10 @@ def is_cached_plan_failure?(e) def load_additional_types(oids = nil) if @config[:use_follower_reads_for_type_introspection] initializer = OID::TypeMapInitializer.new(type_map) - - query = <<~SQL - SELECT t.oid, t.typname, t.typelem, t.typdelim, t.typinput, r.rngsubtype, t.typtype, t.typbasetype - FROM pg_type as t - LEFT JOIN pg_range as r ON oid = rngtypid AS OF SYSTEM TIME '-10s' - SQL - - if oids - query += "WHERE t.oid IN (%s)" % oids.join(", ") - else - query += initializer.query_conditions_for_initial_load - end - - execute_and_clear(query, "SCHEMA", []) do |records| - initializer.run(records) + load_types_queries_with_aost(initializer, oids) do |query| + execute_and_clear(query, "SCHEMA", [], allow_retry: true, materialize_transactions: false) do |records| + initializer.run(records) + end end else super @@ -617,6 +544,21 @@ def load_additional_types(oids = nil) super end + def load_types_queries_with_aost(initializer, oids) + query = <<~SQL + SELECT t.oid, t.typname, t.typelem, t.typdelim, t.typinput, r.rngsubtype, t.typtype, t.typbasetype + FROM pg_type as t + LEFT JOIN pg_range as r ON oid = rngtypid AS OF SYSTEM TIME '-10s' + SQL + if oids + yield query + "WHERE t.oid IN (%s)" % oids.join(", ") + else + yield query + initializer.query_conditions_for_known_type_names + yield query + initializer.query_conditions_for_known_type_types + yield query + initializer.query_conditions_for_array_types + end + end + # override # This method maps data types to their proper decoder. # @@ -647,15 +589,13 @@ def add_pg_decoders WHERE t.typname IN (%s) SQL - coders = execute_and_clear(query, "SCHEMA", []) do |result| - result - .map { |row| construct_coder(row, coders_by_name[row["typname"]]) } - .compact + coders = execute_and_clear(query, "SCHEMA", [], allow_retry: true, materialize_transactions: false) do |result| + result.filter_map { |row| construct_coder(row, coders_by_name[row["typname"]]) } end map = PG::TypeMapByOid.new coders.each { |coder| map.add_coder(coder) } - @connection.type_map_for_results = map + @raw_connection.type_map_for_results = map @type_map_for_results = PG::TypeMapByOid.new @type_map_for_results.default_type_map = map diff --git a/lib/active_record/relation/query_methods_ext.rb b/lib/active_record/relation/query_methods_ext.rb index 65c63698..7b2a6a68 100644 --- a/lib/active_record/relation/query_methods_ext.rb +++ b/lib/active_record/relation/query_methods_ext.rb @@ -123,5 +123,5 @@ def build_from_clause_with_hints # as ancestor. That is how active_record is doing is as well. # # @see https://github.com/rails/rails/blob/914130a9f/activerecord/lib/active_record/querying.rb#L23 - Querying.delegate(:force_index, :index_hint, :aost, to: :all) + Querying.delegate(:force_index, :index_hint, :aost, :show_create, to: :all) end diff --git a/test/cases/adapters/postgresql/connection_test.rb b/test/cases/adapters/postgresql/connection_test.rb new file mode 100644 index 00000000..849cd5a3 --- /dev/null +++ b/test/cases/adapters/postgresql/connection_test.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require "cases/helper_cockroachdb" + + +module ActiveRecord + module CockroachDB + class PostgresqlConnectionTest < ActiveRecord::PostgreSQLTestCase + include ConnectionHelper + + def test_set_session_variable_true + run_without_connection do |orig_connection| + ActiveRecord::Base.establish_connection(orig_connection.deep_merge(variables: { null_ordered_last: true })) + set_true = ActiveRecord::Base.connection.exec_query "SHOW NULL_ORDERED_LAST" + assert_equal [["on"]], set_true.rows + end + end + + def test_set_session_variable_false + run_without_connection do |orig_connection| + ActiveRecord::Base.establish_connection(orig_connection.deep_merge(variables: { null_ordered_last: false })) + set_false = ActiveRecord::Base.connection.exec_query "SHOW NULL_ORDERED_LAST" + assert_equal [["off"]], set_false.rows + end + end + + def test_set_session_variable_nil + run_without_connection do |orig_connection| + # This should be a no-op that does not raise an error + ActiveRecord::Base.establish_connection(orig_connection.deep_merge(variables: { null_ordered_last: nil })) + end + end + + def test_set_session_variable_default + run_without_connection do |orig_connection| + # This should execute a query that does not raise an error + ActiveRecord::Base.establish_connection(orig_connection.deep_merge(variables: { null_ordered_last: :default })) + end + end + end + end +end diff --git a/test/cases/adapters/postgresql/interval_test.rb b/test/cases/adapters/postgresql/interval_test.rb index d34fccce..183e4d17 100644 --- a/test/cases/adapters/postgresql/interval_test.rb +++ b/test/cases/adapters/postgresql/interval_test.rb @@ -30,9 +30,6 @@ def setup assert(@column_min.is_a?(ActiveRecord::ConnectionAdapters::PostgreSQLColumn)) assert_nil @column_max.precision assert_equal 3, @column_min.precision - - crdb_version = @connection.instance_variable_get(:@crdb_version) - @iso8601_enabled = crdb_version >= 2120 end teardown do @@ -94,11 +91,7 @@ def test_interval_type assert_equal "P3Y", i.default_term.iso8601 assert_equal %w[ P1M P1Y PT1H ], i.all_terms.map(&:iso8601) - if @iso8601_enabled - assert_equal "P33Y", i.legacy_term - else - assert_equal "33 years", i.legacy_term - end + assert_equal "P33Y", i.legacy_term end def test_interval_type_cast_from_invalid_string diff --git a/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/test/cases/adapters/postgresql/postgresql_adapter_test.rb index efdb63b6..99417f0f 100644 --- a/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -25,11 +25,11 @@ def teardown end def test_database_exists_returns_false_when_the_database_does_not_exist - db_config = ActiveRecord::Base.configurations.configs_for(env_name: "arunit", name: "primary") - config = db_config.configuration_hash.dup - config[:database] = "non_extant_database" - assert_not ActiveRecord::ConnectionAdapters::CockroachDBAdapter.database_exists?(config), - "expected database #{config[:database]} to not exist" + [ { database: "non_extant_database", adapter: "postgresql" }, + { database: "non_extant_database", adapter: "cockroachdb" } ].each do |config| + assert_not ActiveRecord::ConnectionAdapters::CockroachDBAdapter.database_exists?(config), + "expected database #{config[:database]} to not exist" + end end def test_database_exists_returns_true_when_the_database_exists @@ -63,23 +63,34 @@ def test_using_follower_reads_connects_properly assert conn_config[:use_follower_reads_for_type_introspection] end - def test_only_reload_type_map_once_for_every_unrecognized_type - reset_connection - connection = ActiveRecord::Base.connection + # OVERRIDE: CockroachDB adds parentheses around the WHERE clause's content. + def test_partial_index_on_column_named_like_keyword + with_example_table('id serial primary key, number integer, "primary" boolean') do + @connection.add_index "ex", "id", name: "partial", where: "primary" # "primary" is a keyword + index = @connection.indexes("ex").find { |idx| idx.name == "partial" } + assert_equal '("primary")', index.where + end + end - silence_stream($stdout) do - assert_queries 2, ignore_none: true do - connection.select_all "select 'pg_catalog.pg_class'::regclass" - end - assert_queries 1, ignore_none: true do - connection.select_all "select 'pg_catalog.pg_class'::regclass" - end - assert_queries 2, ignore_none: true do - connection.select_all "SELECT NULL::anyarray" + # OVERRIDE: Different behaviour between PostgreSQL and CockroachDB. + def test_invalid_index + with_example_table do + @connection.exec_query("INSERT INTO ex (number) VALUES (1), (1)") + error = assert_raises(ActiveRecord::RecordNotUnique) do + @connection.add_index(:ex, :number, unique: true, algorithm: :concurrently, name: :invalid_index) end + assert_match(/duplicate key value violates unique constraint/, error.message) + assert_equal @connection.pool, error.connection_pool + + # In CRDB this tests won't create the index at all. + assert_not @connection.index_exists?(:ex, :number, name: :invalid_index) end - ensure - reset_connection + end + + private + + def with_example_table(definition = "id serial primary key, number integer, data character varying(255)", &block) + super(@connection, "ex", definition, &block) end end end diff --git a/test/cases/adapters/postgresql/virtual_column_test.rb b/test/cases/adapters/postgresql/virtual_column_test.rb index de8073bb..f4546557 100644 --- a/test/cases/adapters/postgresql/virtual_column_test.rb +++ b/test/cases/adapters/postgresql/virtual_column_test.rb @@ -3,18 +3,37 @@ require "cases/helper" require "support/schema_dumping_helper" -if ActiveRecord::Base.connection.supports_virtual_columns? - class PostgresqlVirtualColumnTest < ActiveRecord::PostgreSQLTestCase - include SchemaDumpingHelper +module CockroachDB + if ActiveRecord::Base.connection.supports_virtual_columns? + class PostgresqlVirtualColumnTest < ActiveRecord::PostgreSQLTestCase + include SchemaDumpingHelper - self.use_transactional_tests = false + self.use_transactional_tests = false - def test_schema_dumping - output = dump_table_schema("virtual_columns") - assert_match(/t\.virtual\s+"upper_name",\s+type: :string,\s+as: nil, stored: true$/i, output) - assert_match(/t\.virtual\s+"name_length",\s+type: :integer,\s+as: "length\(\(name\)::text\)", stored: true$/i, output) - assert_match(/t\.virtual\s+"name_octet_length",\s+type: :integer,\s+as: "octet_length\(\(name\)::text\)", stored: true$/i, output) - assert_match(/t\.virtual\s+"column2",\s+type: :integer,\s+as: "\(column1 \+ 1\)", stored: true$/i, output) + class VirtualColumn < ActiveRecord::Base + end + + def setup + @connection = ActiveRecord::Base.connection + @connection.create_table :virtual_columns, force: true do |t| + t.string :name + t.virtual :upper_name, type: :string, as: "UPPER(name)", stored: true + t.virtual :name_length, type: :integer, as: "LENGTH(name)", stored: true + t.virtual :name_octet_length, type: :integer, as: "OCTET_LENGTH(name)", stored: true + t.integer :column1 + t.virtual :column2, type: :integer, as: "column1 + 1", stored: true + end + VirtualColumn.create(name: "Rails") + end + + # TODO: is this test result acceptable? + def test_schema_dumping + output = dump_table_schema("virtual_columns") + assert_match(/t\.virtual\s+"upper_name",\s+type: :string,\s+as: "upper\(name\)", stored: true$/i, output) + assert_match(/t\.virtual\s+"name_length",\s+type: :bigint,\s+as: "length\(name\)", stored: true$/i, output) + assert_match(/t\.virtual\s+"name_octet_length",\s+type: :bigint,\s+as: "octet_length\(name\)", stored: true$/i, output) + assert_match(/t\.virtual\s+"column2",\s+type: :bigint,\s+as: "column1 \+ 1", stored: true$/i, output) + end end end end diff --git a/test/cases/arel/nodes_test.rb b/test/cases/arel/nodes_test.rb deleted file mode 100644 index 98837095..00000000 --- a/test/cases/arel/nodes_test.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true -# This file may be remove after next -# rails release. -# Whenever https://github.com/rails/rails/commit/8bd463c -# is part of a rails version. - -require "cases/arel/helper" - -module Arel - module Nodes - class TestNodes < Arel::Test - def test_every_arel_nodes_have_hash_eql_eqeq_from_same_class - # #descendants code from activesupport - node_descendants = [] - ObjectSpace.each_object(Arel::Nodes::Node.singleton_class) do |k| - next if k.respond_to?(:singleton_class?) && k.singleton_class? - node_descendants.unshift k unless k == self - end - node_descendants.delete(Arel::Nodes::Node) - node_descendants.delete(Arel::Nodes::NodeExpression) - - default_hash_owner = Object.instance_method(:hash).owner - - bad_node_descendants = node_descendants.reject do |subnode| - eqeq_owner = subnode.instance_method(:==).owner - eql_owner = subnode.instance_method(:eql?).owner - hash_owner = subnode.instance_method(:hash).owner - - hash_owner != default_hash_owner && - eqeq_owner == eql_owner && - eqeq_owner == hash_owner - end - - problem_msg = "Some subclasses of Arel::Nodes::Node do not have a" \ - " #== or #eql? or #hash defined from the same class as the others" - assert_empty bad_node_descendants, problem_msg - end - end - end -end diff --git a/test/cases/connection_adapters/schema_cache_test.rb b/test/cases/connection_adapters/schema_cache_test.rb deleted file mode 100644 index 663a3d5a..00000000 --- a/test/cases/connection_adapters/schema_cache_test.rb +++ /dev/null @@ -1,74 +0,0 @@ -require "cases/helper_cockroachdb" - -module CockroachDB - module ConnectionAdapters - class SchemaCacheTest < ActiveRecord::TestCase - def setup - @connection = ActiveRecord::Base.connection - @database_version = @connection.get_database_version - end - - # This replaces the same test that's been excluded from - # ActiveRecord::ConnectionAdapters::SchemaCacheTest. It's exactly the - # same, but we can run it here by fixing schema_dump_path so it has a - # valid path. - # See test/excludes/ActiveRecord/ConnectionAdapters/SchemaCacheTest.rb - def test_yaml_loads_5_1_dump - body = File.open(schema_dump_path).read - cache = YAML.unsafe_load(body) - - assert_no_queries do - assert_equal 11, cache.columns("posts").size - assert_equal 11, cache.columns_hash("posts").size - assert cache.data_sources("posts") - assert_equal "id", cache.primary_keys("posts") - end - end - - # This replaces the same test that's been excluded from - # ActiveRecord::ConnectionAdapters::SchemaCacheTest. It's exactly the - # same, but we can run it here by fixing schema_dump_path so it has a - # valid path. - # See test/excludes/ActiveRecord/ConnectionAdapters/SchemaCacheTest.rb - def test_yaml_loads_5_1_dump_without_indexes_still_queries_for_indexes - body = File.open(schema_dump_path).read - @cache = YAML.unsafe_load(body) - - # Simulate assignment in railtie after loading the cache. - old_cache, @connection.schema_cache = @connection.schema_cache, @cache - - assert_queries :any, ignore_none: true do - assert_equal 1, @cache.indexes("posts").size - end - ensure - @connection.schema_cache = old_cache - end - - # This replaces the same test that's been excluded from - # ActiveRecord::ConnectionAdapters::SchemaCacheTest. It's exactly the - # same, but we can run it here by fixing schema_dump_path so it has a - # valid path. - # See test/excludes/ActiveRecord/ConnectionAdapters/SchemaCacheTest.rb - def test_yaml_loads_5_1_dump_without_database_version_still_queries_for_database_version - body = File.open(schema_dump_path).read - @cache = YAML.unsafe_load(body) - - # Simulate assignment in railtie after loading the cache. - old_cache, @connection.schema_cache = @connection.schema_cache, @cache - - # We can't verify queries get executed because the database version gets - # cached in both MySQL and PostgreSQL outside of the schema cache. - assert_nil @cache.instance_variable_get(:@database_version) - assert_equal @database_version.to_s, @cache.database_version.to_s - ensure - @connection.schema_cache = old_cache - end - - private - - def schema_dump_path - "#{ASSETS_ROOT}/schema_dump_5_1.yml" - end - end - end -end diff --git a/test/cases/connection_adapters/type_test.rb b/test/cases/connection_adapters/type_test.rb index dd1e5eff..8b47004a 100644 --- a/test/cases/connection_adapters/type_test.rb +++ b/test/cases/connection_adapters/type_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "cases/helper_cockroachdb" require "models/account" diff --git a/test/cases/enum_test.rb b/test/cases/enum_test.rb deleted file mode 100644 index 18dac017..00000000 --- a/test/cases/enum_test.rb +++ /dev/null @@ -1,96 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" -require "models/author" -require "models/book" -require "active_support/log_subscriber/test_helper" - -module CockroachDB - class EnumTest < ActiveRecord::TestCase - fixtures :books, :authors, :author_addresses - - setup do - @book = books(:awdr) - end - - test "enum logs a warning if auto-generated negative scopes would clash with other enum names" do - old_logger = ActiveRecord::Base.logger - logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new - - ActiveRecord::Base.logger = logger - - expected_message_1 = "Enum element 'not_sent' in Book uses the prefix 'not_'."\ - " This has caused a conflict with auto generated negative scopes."\ - " Avoid using enum elements starting with 'not' where the positive form is also an element." - - Class.new(ActiveRecord::Base) do - def self.name - "Book" - end - enum status: [:sent, :not_sent] - end - - assert_includes(logger.logged(:warn), expected_message_1) - ensure - ActiveRecord::Base.logger = old_logger - end - - test "enum logs a warning if auto-generated negative scopes would clash with other enum names regardless of order" do - old_logger = ActiveRecord::Base.logger - logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new - - ActiveRecord::Base.logger = logger - - expected_message_1 = "Enum element 'not_sent' in Book uses the prefix 'not_'."\ - " This has caused a conflict with auto generated negative scopes."\ - " Avoid using enum elements starting with 'not' where the positive form is also an element." - - Class.new(ActiveRecord::Base) do - def self.name - "Book" - end - enum status: [:not_sent, :sent] - end - - assert_includes(logger.logged(:warn), expected_message_1) - ensure - ActiveRecord::Base.logger = old_logger - end - - test "enum doesn't log a warning if no clashes detected" do - old_logger = ActiveRecord::Base.logger - logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new - - ActiveRecord::Base.logger = logger - - Class.new(ActiveRecord::Base) do - def self.name - "Book" - end - enum status: [:not_sent] - end - - assert_empty(logger.logged(:warn)) - ensure - ActiveRecord::Base.logger = old_logger - end - - test "enum doesn't log a warning if opting out of scopes" do - old_logger = ActiveRecord::Base.logger - logger = ActiveSupport::LogSubscriber::TestHelper::MockLogger.new - - ActiveRecord::Base.logger = logger - - Class.new(ActiveRecord::Base) do - def self.name - "Book" - end - enum status: [:not_sent, :sent], _scopes: false - end - - assert_empty(logger.logged(:warn)) - ensure - ActiveRecord::Base.logger = old_logger - end - end -end diff --git a/test/cases/fixtures_test.rb b/test/cases/fixtures_test.rb index db52d24a..e21fedb9 100644 --- a/test/cases/fixtures_test.rb +++ b/test/cases/fixtures_test.rb @@ -310,6 +310,8 @@ def before_setup Account.connection.exec_query(" CREATE TABLE accounts ( id BIGINT PRIMARY KEY DEFAULT nextval('accounts_id_seq'), + created_at timestamp NULL, + updated_at timestamp NULL, firm_id bigint, firm_name character varying, credit_limit integer, @@ -360,6 +362,7 @@ def teardown Account.connection.drop_table :accounts, if_exists: true Account.connection.exec_query("DROP SEQUENCE IF EXISTS accounts_id_seq") Account.connection.create_table :accounts, force: true do |t| + t.timestamps null: true t.references :firm, index: false t.string :firm_name t.integer :credit_limit @@ -370,7 +373,7 @@ def teardown Company.connection.drop_table :companies, if_exists: true Company.connection.exec_query("DROP SEQUENCE IF EXISTS companies_nonstd_seq") Company.connection.create_table :companies, force: true do |t| - t.string :type + t.string :type t.references :firm, index: false t.string :firm_name t.string :name @@ -383,15 +386,17 @@ def teardown t.index [:name, :description], length: 10 t.index [:firm_id, :type, :rating], name: "company_index", length: { type: 10 }, order: { rating: :desc } t.index [:firm_id, :type], name: "company_partial_index", where: "(rating > 10)" + t.index [:firm_id], name: "company_nulls_not_distinct", nulls_not_distinct: true t.index :name, name: "company_name_index", using: :btree - t.index "(CASE WHEN rating > 0 THEN lower(name) END)", name: "company_expression_index" if Company.connection.supports_expression_index? + t.index "(CASE WHEN rating > 0 THEN lower(name) END) DESC", name: "company_expression_index" if Company.connection.supports_expression_index? + t.index [:firm_id, :type], name: "company_include_index", include: [:name, :account_id] end Course.connection.drop_table :courses, if_exists: true Course.connection.exec_query("DROP SEQUENCE IF EXISTS courses_id_seq") Course.connection.create_table :courses, force: true do |t| t.column :name, :string, null: false - t.column :college_id, :integer + t.column :college_id, :integer, index: true end recreate_parrots diff --git a/test/cases/helper.rb b/test/cases/helper.rb deleted file mode 100644 index 754f1a13..00000000 --- a/test/cases/helper.rb +++ /dev/null @@ -1,276 +0,0 @@ -# frozen_string_literal: true - -# This is copied directly from activerecord/test/cases/helper.rb -# The only difference is, we decide whether to load_schema or do -# a RESTORE based on an environment variable. - -require "config" - -require "stringio" - -require "active_record" -require "cases/test_case" -require "active_support/dependencies" -require "active_support/logger" -require "active_support/core_ext/kernel/reporting" -require "active_support/core_ext/kernel/singleton_class" - -require "support/config" -require "support/connection" - -# TODO: Move all these random hacks into the ARTest namespace and into the support/ dir - -Thread.abort_on_exception = true - -# Show backtraces for deprecated behavior for quicker cleanup. -ActiveSupport::Deprecation.debug = true - -# Disable available locale checks to avoid warnings running the test suite. -I18n.enforce_available_locales = false - -# Connect to the database -ARTest.connect - -# Quote "type" if it's a reserved word for the current connection. -QUOTED_TYPE = ActiveRecord::Base.connection.quote_column_name("type") - -def current_adapter?(*types) - types.any? do |type| - ActiveRecord::ConnectionAdapters.const_defined?(type) && - ActiveRecord::Base.connection.is_a?(ActiveRecord::ConnectionAdapters.const_get(type)) - end -end - -def in_memory_db? - current_adapter?(:SQLite3Adapter) && - ActiveRecord::Base.connection_pool.db_config.database == ":memory:" -end - -def mysql_enforcing_gtid_consistency? - current_adapter?(:Mysql2Adapter) && "ON" == ActiveRecord::Base.connection.show_variable("enforce_gtid_consistency") -end - -def supports_default_expression? - if current_adapter?(:PostgreSQLAdapter) - true - elsif current_adapter?(:Mysql2Adapter) - conn = ActiveRecord::Base.connection - !conn.mariadb? && conn.database_version >= "8.0.13" - end -end - -def supports_non_unique_constraint_name? - if current_adapter?(:Mysql2Adapter) - conn = ActiveRecord::Base.connection - conn.mariadb? - else - false - end -end - -def supports_text_column_with_default? - if current_adapter?(:Mysql2Adapter) - conn = ActiveRecord::Base.connection - conn.mariadb? && conn.database_version >= "10.2.1" - else - true - end -end - -%w[ - supports_savepoints? - supports_partial_index? - supports_partitioned_indexes? - supports_expression_index? - supports_insert_returning? - supports_insert_on_duplicate_skip? - supports_insert_on_duplicate_update? - supports_insert_conflict_target? - supports_optimizer_hints? - supports_datetime_with_precision? -].each do |method_name| - define_method method_name do - ActiveRecord::Base.connection.public_send(method_name) - end -end - -def with_env_tz(new_tz = "US/Eastern") - old_tz, ENV["TZ"] = ENV["TZ"], new_tz - yield -ensure - old_tz ? ENV["TZ"] = old_tz : ENV.delete("TZ") -end - -def with_timezone_config(cfg) - verify_default_timezone_config - - old_default_zone = ActiveRecord.default_timezone - old_awareness = ActiveRecord::Base.time_zone_aware_attributes - old_aware_types = ActiveRecord::Base.time_zone_aware_types - old_zone = Time.zone - - if cfg.has_key?(:default) - ActiveRecord.default_timezone = cfg[:default] - end - if cfg.has_key?(:aware_attributes) - ActiveRecord::Base.time_zone_aware_attributes = cfg[:aware_attributes] - end - if cfg.has_key?(:aware_types) - ActiveRecord::Base.time_zone_aware_types = cfg[:aware_types] - end - if cfg.has_key?(:zone) - Time.zone = cfg[:zone] - end - yield -ensure - ActiveRecord.default_timezone = old_default_zone - ActiveRecord::Base.time_zone_aware_attributes = old_awareness - ActiveRecord::Base.time_zone_aware_types = old_aware_types - Time.zone = old_zone -end - -# This method makes sure that tests don't leak global state related to time zones. -EXPECTED_ZONE = nil -EXPECTED_DEFAULT_TIMEZONE = :utc -EXPECTED_AWARE_TYPES = [:datetime, :time] -EXPECTED_TIME_ZONE_AWARE_ATTRIBUTES = false -def verify_default_timezone_config - if Time.zone != EXPECTED_ZONE - $stderr.puts <<-MSG -\n#{self} - Global state `Time.zone` was leaked. - Expected: #{EXPECTED_ZONE} - Got: #{Time.zone} - MSG - end - if ActiveRecord.default_timezone != EXPECTED_DEFAULT_TIMEZONE - $stderr.puts <<-MSG -\n#{self} - Global state `ActiveRecord.default_timezone` was leaked. - Expected: #{EXPECTED_DEFAULT_TIMEZONE} - Got: #{ActiveRecord.default_timezone} - MSG - end - if ActiveRecord::Base.time_zone_aware_attributes != EXPECTED_TIME_ZONE_AWARE_ATTRIBUTES - $stderr.puts <<-MSG -\n#{self} - Global state `ActiveRecord::Base.time_zone_aware_attributes` was leaked. - Expected: #{EXPECTED_TIME_ZONE_AWARE_ATTRIBUTES} - Got: #{ActiveRecord::Base.time_zone_aware_attributes} - MSG - end - if ActiveRecord::Base.time_zone_aware_types != EXPECTED_AWARE_TYPES - $stderr.puts <<-MSG -\n#{self} - Global state `ActiveRecord::Base.time_zone_aware_types` was leaked. - Expected: #{EXPECTED_AWARE_TYPES} - Got: #{ActiveRecord::Base.time_zone_aware_types} - MSG - end -end - -def enable_extension!(extension, connection) - return false unless connection.supports_extensions? - return connection.reconnect! if connection.extension_enabled?(extension) - - connection.enable_extension extension - connection.commit_db_transaction if connection.transaction_open? - connection.reconnect! -end - -def disable_extension!(extension, connection) - return false unless connection.supports_extensions? - return true unless connection.extension_enabled?(extension) - - connection.disable_extension extension - connection.reconnect! -end - -def clean_up_legacy_connection_handlers - handler = ActiveRecord::Base.default_connection_handler - assert_deprecated do - ActiveRecord::Base.connection_handlers = {} - end - - handler.connection_pool_names.each do |name| - next if ["ActiveRecord::Base", "ARUnit2Model", "Contact", "ContactSti", "FirstAbstractClass", "SecondAbstractClass"].include?(name) - - handler.send(:owner_to_pool_manager).delete(name) - end -end - -def clean_up_connection_handler - handler = ActiveRecord::Base.connection_handler - handler.instance_variable_get(:@owner_to_pool_manager).each do |owner, pool_manager| - pool_manager.role_names.each do |role_name| - next if role_name == ActiveRecord::Base.default_role - pool_manager.remove_role(role_name) - end - end -end - -def load_schema(shush = true) - if shush - # silence verbose schema loading - original_stdout = $stdout - $stdout = StringIO.new - end - - adapter_name = ActiveRecord::Base.connection.adapter_name.downcase - adapter_specific_schema_file = SCHEMA_ROOT + "/#{adapter_name}_specific_schema.rb" - - load SCHEMA_ROOT + "/schema.rb" - - if File.exist?(adapter_specific_schema_file) - load adapter_specific_schema_file - end - - ActiveRecord::FixtureSet.reset_cache -ensure - $stdout = original_stdout if shush -end - -if ENV['COCKROACH_LOAD_FROM_TEMPLATE'].nil? && ENV['COCKROACH_SKIP_LOAD_SCHEMA'].nil? - load_schema -end -class SQLSubscriber - attr_reader :logged - attr_reader :payloads - - def initialize - @logged = [] - @payloads = [] - end - - def start(name, id, payload) - @payloads << payload - @logged << [payload[:sql].squish, payload[:name], payload[:binds]] - end - - def finish(name, id, payload); end -end - -module InTimeZone - private - def in_time_zone(zone) - old_zone = Time.zone - old_tz = ActiveRecord::Base.time_zone_aware_attributes - - Time.zone = zone ? ActiveSupport::TimeZone[zone] : nil - ActiveRecord::Base.time_zone_aware_attributes = !zone.nil? - yield - ensure - Time.zone = old_zone - ActiveRecord::Base.time_zone_aware_attributes = old_tz - end -end - -# Encryption - -ActiveRecord::Encryption.configure \ - primary_key: "test master key", - deterministic_key: "test deterministic key", - key_derivation_salt: "testing key derivation salt" - -ActiveRecord::Encryption::ExtendedDeterministicQueries.install_support -ActiveRecord::Encryption::ExtendedDeterministicUniquenessValidator.install_support diff --git a/test/cases/helper_cockroachdb.rb b/test/cases/helper_cockroachdb.rb index 2bcbfe24..eb9b5f21 100644 --- a/test/cases/helper_cockroachdb.rb +++ b/test/cases/helper_cockroachdb.rb @@ -1,11 +1,32 @@ require 'bundler' Bundler.setup +CRDB_VALIDATE_BUG = "CockcroachDB bug, see https://github.com/cockroachdb/cockroach/blob/dd1e0e0164cb3d5859ea4bb23498863d1eebc0af/pkg/sql/alter_table.go#L458-L467" require "minitest/excludes" +require "minitest/github_action_reporter" + +# This gives great visibility on schema dump related tests, but +# 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 +# skip this step +require "support/load_schema_helper" +module LoadSchemaHelperExt + def load_schema + return if ENV['COCKROACH_LOAD_FROM_TEMPLATE'] + return if ENV['COCKROACH_SKIP_LOAD_SCHEMA'] + + super + end +end +LoadSchemaHelper.prepend(LoadSchemaHelperExt) + # Load ActiveRecord test helper require "cases/helper" @@ -117,3 +138,41 @@ def with_cockroachdb_datetime_type(type) end ActiveRecord::TestCase.include(ARTestCaseHelper) + +module Minitest + module GithubActionReporterExt + def gh_link(loc) + return super unless loc.include?("/gems/") + + path, _, line = loc[%r(/(?:test|spec|lib)/.*)][1..].rpartition(":") + + rails_version = "v#{ActiveRecord::VERSION::STRING}" + "#{ENV["GITHUB_SERVER_URL"]}/rails/rails/blob/#{rails_version}/activerecord/#{path}#L#{line}" + rescue + warn "Failed to generate link for #{loc}" + super + end + end + GithubActionReporter.prepend(GithubActionReporterExt) +end + +if ENV['AR_LOG'] + ActiveRecord::Base.logger = Logger.new(STDOUT) + ActiveRecord::Base.logger.level = Logger::DEBUG + ActiveRecord::Base.logger.formatter = proc { |severity, time, progname, msg| + th = Thread.current[:name] + th = "THREAD=#{th}" if th + Logger::Formatter.new.call(severity, time, progname || th, msg) + } +end + +# Remove the header from the schema dump to clarify tests outputs. +module NoHeaderExt + def header(stream) + with_comments = StringIO.new + super(with_comments) + stream.print with_comments.string.gsub(/^(:?#.*)?\n/, '') + end +end + +ActiveRecord::SchemaDumper.prepend(NoHeaderExt) diff --git a/test/cases/migration/check_constraint_test.rb b/test/cases/migration/check_constraint_test.rb index 4bfc8025..7511c86b 100644 --- a/test/cases/migration/check_constraint_test.rb +++ b/test/cases/migration/check_constraint_test.rb @@ -37,6 +37,23 @@ def test_validate_check_constraint_by_name end end + # CRDB_VALIDATE_BUG + # def test_schema_dumping_with_validate_false + # @connection.add_check_constraint :trades, "quantity > 0", name: "quantity_check", validate: false + + # output = dump_table_schema "trades" + + # assert_match %r{\s+t.check_constraint "(quantity > 0)", name: "quantity_check", validate: false$}, output + # end + + def test_schema_dumping_with_validate_true + @connection.add_check_constraint :trades, "quantity > 0", name: "quantity_check", validate: true + + output = dump_table_schema "trades" + + assert_match %r{\s+t.check_constraint "\(quantity > 0\)", name: "quantity_check"$}, output + end + # keep def test_remove_check_constraint @connection.add_check_constraint :trades, "price > 0", name: "price_check" diff --git a/test/cases/migration/foreign_key_test.rb b/test/cases/migration/foreign_key_test.rb index c722ec07..e9274e5d 100644 --- a/test/cases/migration/foreign_key_test.rb +++ b/test/cases/migration/foreign_key_test.rb @@ -232,13 +232,6 @@ def test_schema_dumping assert_match %r{\s+add_foreign_key "astronauts", "rockets"$}, output end - def test_schema_dumping_on_delete_and_on_update_options - @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", on_delete: :nullify, on_update: :cascade - - output = dump_table_schema "astronauts" - assert_match %r{\s+add_foreign_key "astronauts",.+on_update: :cascade,.+on_delete: :nullify$}, output - end - class CreateCitiesAndHousesMigration < ActiveRecord::Migration::Current def change create_table("cities") { |t| } @@ -365,6 +358,36 @@ def test_add_foreign_key_with_if_not_exists_set end end end + + class CompositeForeignKeyTest < ActiveRecord::TestCase + include SchemaDumpingHelper + + setup do + @connection = ActiveRecord::Base.connection + @connection.create_table :rockets, primary_key: [:tenant_id, :id], force: true do |t| + t.integer :tenant_id + t.integer :id + end + @connection.create_table :astronauts, force: true do |t| + t.integer :rocket_id + t.integer :rocket_tenant_id + end + end + + teardown do + @connection.drop_table :astronauts, if_exists: true rescue nil + @connection.drop_table :rockets, if_exists: true rescue nil + end + + # OVERRIDE: CockroachDB does not quote the table name. + def test_add_composite_foreign_key_raises_without_options + error = assert_raises(ActiveRecord::StatementInvalid) do + @connection.add_foreign_key :astronauts, :rockets + end + + assert_match(/there is no unique constraint matching given keys for referenced table rockets/, error.message) + end + end end end end diff --git a/test/cases/migration/index_test.rb b/test/cases/migration/index_test.rb deleted file mode 100644 index d73c6b46..00000000 --- a/test/cases/migration/index_test.rb +++ /dev/null @@ -1,233 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" - -module ActiveRecord - module CockroachDB - class Migration - class IndexTest < ActiveRecord::TestCase - # These tests are identical to the ones found in Rails, save for the fact - # that transactions are turned off for test runs. It is necessary to disable - # transactional tests in order to assert on schema changes due to how - # CockroachDB handles transactions. - self.use_transactional_tests = false - - attr_reader :connection, :table_name - - def setup - super - @connection = ActiveRecord::Base.connection - @table_name = :testings - - connection.create_table table_name do |t| - t.column :foo, :string, limit: 100 - t.column :bar, :string, limit: 100 - - t.string :first_name - t.string :last_name, limit: 100 - t.string :key, limit: 100 - t.boolean :administrator - end - end - - teardown do - connection.drop_table :testings rescue nil - ActiveRecord::Base.primary_key_prefix_type = nil - end - - def test_rename_index - # keep the names short to make Oracle and similar behave - connection.add_index(table_name, [:foo], name: "old_idx") - connection.rename_index(table_name, "old_idx", "new_idx") - - assert_not connection.index_name_exists?(table_name, "old_idx") - assert connection.index_name_exists?(table_name, "new_idx") - end - - def test_rename_index_too_long - too_long_index_name = good_index_name + "x" - # keep the names short to make Oracle and similar behave - connection.add_index(table_name, [:foo], name: "old_idx") - e = assert_raises(ArgumentError) { - connection.rename_index(table_name, "old_idx", too_long_index_name) - } - assert_match(/too long; the limit is #{connection.index_name_length} characters/, e.message) - - assert connection.index_name_exists?(table_name, "old_idx") - end - - def test_double_add_index - connection.add_index(table_name, [:foo], name: "some_idx") - assert_raises(ActiveRecord::StatementInvalid) { - connection.add_index(table_name, [:foo], name: "some_idx") - } - end - - def test_add_index_works_with_long_index_names - connection.add_index(table_name, "foo", name: good_index_name) - - assert connection.index_name_exists?(table_name, good_index_name) - connection.remove_index(table_name, name: good_index_name) - end - - def test_internal_index_with_name_matching_database_limit - good_index_name = "x" * connection.index_name_length - connection.add_index(table_name, "foo", name: good_index_name, internal: true) - - assert connection.index_name_exists?(table_name, good_index_name) - connection.remove_index(table_name, name: good_index_name) - end - - def test_index_symbol_names - connection.add_index table_name, :foo, name: :symbol_index_name - assert connection.index_exists?(table_name, :foo, name: :symbol_index_name) - - connection.remove_index table_name, name: :symbol_index_name - assert_not connection.index_exists?(table_name, :foo, name: :symbol_index_name) - end - - def test_index_exists - connection.add_index :testings, :foo - - assert connection.index_exists?(:testings, :foo) - assert !connection.index_exists?(:testings, :bar) - end - - def test_index_exists_on_multiple_columns - connection.add_index :testings, [:foo, :bar] - - assert connection.index_exists?(:testings, [:foo, :bar]) - end - - def test_index_exists_with_custom_name_checks_columns - connection.add_index :testings, [:foo, :bar], name: "my_index" - assert connection.index_exists?(:testings, [:foo, :bar], name: "my_index") - assert_not connection.index_exists?(:testings, [:foo], name: "my_index") - end - - def test_unique_index_exists - connection.add_index :testings, :foo, unique: true - - assert connection.index_exists?(:testings, :foo, unique: true) - end - - def test_named_index_exists - connection.add_index :testings, :foo, name: "custom_index_name" - - assert connection.index_exists?(:testings, :foo) - assert connection.index_exists?(:testings, :foo, name: "custom_index_name") - assert !connection.index_exists?(:testings, :foo, name: "other_index_name") - end - - def test_remove_named_index - connection.add_index :testings, :foo, name: "custom_index_name" - - assert connection.index_exists?(:testings, :foo) - connection.remove_index :testings, :foo - assert !connection.index_exists?(:testings, :foo) - end - - def test_add_index_attribute_length_limit - connection.add_index :testings, [:foo, :bar], length: { foo: 10, bar: nil } - - assert connection.index_exists?(:testings, [:foo, :bar]) - end - - def test_add_index - connection.add_index("testings", "last_name") - connection.remove_index("testings", "last_name") - - connection.add_index("testings", ["last_name", "first_name"]) - connection.remove_index("testings", column: ["last_name", "first_name"]) - - # Oracle adapter cannot have specified index name larger than 30 characters - # Oracle adapter is shortening index name when just column list is given - unless current_adapter?(:OracleAdapter) - connection.add_index("testings", ["last_name", "first_name"]) - connection.remove_index("testings", name: :index_testings_on_last_name_and_first_name) - connection.add_index("testings", ["last_name", "first_name"]) - connection.remove_index("testings", "last_name_and_first_name") - end - connection.add_index("testings", ["last_name", "first_name"]) - connection.remove_index("testings", ["last_name", "first_name"]) - - connection.add_index("testings", ["last_name"], length: 10) - connection.remove_index("testings", "last_name") - - connection.add_index("testings", ["last_name"], length: { last_name: 10 }) - connection.remove_index("testings", ["last_name"]) - - connection.add_index("testings", ["last_name", "first_name"], length: 10) - connection.remove_index("testings", ["last_name", "first_name"]) - - connection.add_index("testings", ["last_name", "first_name"], length: { last_name: 10, first_name: 20 }) - connection.remove_index("testings", ["last_name", "first_name"]) - - connection.add_index("testings", ["key"], name: "key_idx", unique: true) - connection.remove_index("testings", name: "key_idx", unique: true) - - connection.add_index("testings", %w(last_name first_name administrator), name: "named_admin") - connection.remove_index("testings", name: "named_admin") - - # Test support for index sort order - connection.add_index("testings", ["last_name"], order: { last_name: :desc }) - connection.remove_index("testings", ["last_name"]) - connection.add_index("testings", ["last_name", "first_name"], order: { last_name: :desc }) - connection.remove_index("testings", ["last_name", "first_name"]) - connection.add_index("testings", ["last_name", "first_name"], order: { last_name: :desc, first_name: :asc }) - connection.remove_index("testings", ["last_name", "first_name"]) - connection.add_index("testings", ["last_name", "first_name"], order: :desc) - connection.remove_index("testings", ["last_name", "first_name"]) - end - - def test_add_partial_index - connection.add_index("testings", "last_name", where: "first_name = 'john doe'") - assert connection.index_exists?("testings", "last_name") - - connection.remove_index("testings", "last_name") - assert_not connection.index_exists?("testings", "last_name") - end - - def test_add_index_with_if_not_exists_matches_exact_index - connection.add_index(table_name, [:foo, :bar], unique: false, name: "index_testings_on_foo_bar") - - assert connection.index_name_exists?(table_name, "index_testings_on_foo_bar") - assert_nothing_raised do - res = connection.add_index(table_name, [:foo, :bar], unique: true, if_not_exists: true) - end - assert connection.index_name_exists?(table_name, "index_testings_on_foo_and_bar") - end - - def test_remove_index_with_name_which_does_not_exist_doesnt_raise_with_option - connection.add_index(table_name, [:foo], name: "foo") - - assert connection.index_exists?(table_name, :foo, name: "foo") - - connection.remove_index(table_name, nil, name: "foo", if_exists: true) - - assert_not connection.index_exists?(table_name, :foo, name: "foo") - end - - def test_remove_index_which_does_not_exist_doesnt_raise_with_option - connection.add_index(table_name, "foo") - - connection.remove_index(table_name, "foo") - - assert_raises ArgumentError do - connection.remove_index(table_name, "foo") - end - - assert_nothing_raised do - connection.remove_index(table_name, "foo", if_exists: true) - end - end - - private - def good_index_name - "x" * connection.index_name_length - end - - end - end - end -end diff --git a/test/cases/migration/unique_constraint_test.rb b/test/cases/migration/unique_constraint_test.rb new file mode 100644 index 00000000..a514a410 --- /dev/null +++ b/test/cases/migration/unique_constraint_test.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require "cases/helper_cockroachdb" +require "support/schema_dumping_helper" + + +module ActiveRecord + module Cockroach + class Migration + class UniqueConstraintTest < ActiveRecord::TestCase + + setup do + @connection = ActiveRecord::Base.connection + @connection.create_table "sections", force: true do |t| + t.integer "position", null: false + end + end + + teardown do + @connection.drop_table "sections", if_exists: true + end + + def test_unique_constraints + unique_constraints = @connection.unique_constraints("test_unique_constraints") + + expected_constraints = [ + { + name: "test_unique_constraints_position_1", + column: ["position_1"] + }, { + name: "test_unique_constraints_position_2", + column: ["position_2"] + }, { + name: "test_unique_constraints_position_3", + column: ["position_3"] + } + ] + + assert_equal expected_constraints.size, unique_constraints.size + + expected_constraints.each do |expected_constraint| + constraint = unique_constraints.find { |constraint| constraint.name == expected_constraint[:name] } + assert_equal "test_unique_constraints", constraint.table_name + assert_equal expected_constraint[:name], constraint.name + assert_equal expected_constraint[:column], constraint.column + end + end + end + end + end +end diff --git a/test/cases/migration_test.rb b/test/cases/migration_test.rb index de516cfa..05e74617 100644 --- a/test/cases/migration_test.rb +++ b/test/cases/migration_test.rb @@ -17,6 +17,7 @@ def setup Reminder.reset_column_information @verbose_was, ActiveRecord::Migration.verbose = ActiveRecord::Migration.verbose, false @schema_migration = ActiveRecord::Base.connection.schema_migration + @internal_metadata = ActiveRecord::Base.connection.internal_metadata ActiveRecord::Base.connection.schema_cache.clear! end @@ -24,8 +25,8 @@ def setup ActiveRecord::Base.table_name_prefix = "" ActiveRecord::Base.table_name_suffix = "" - ActiveRecord::SchemaMigration.create_table - ActiveRecord::SchemaMigration.delete_all + @schema_migration.create_table + @schema_migration.delete_all_versions %w(things awesome_things prefix_things_suffix p_awesome_things_s).each do |table| Thing.connection.drop_table(table) rescue nil @@ -72,12 +73,12 @@ def migrate(x) end }.new - ActiveRecord::Migrator.new(:up, [migration_a], @schema_migration, 100).migrate + ActiveRecord::Migrator.new(:up, [migration_a], @schema_migration, @internal_metadata, 100).migrate assert_column Person, :last_name, "migration_a should have added the last_name column on people" - ActiveRecord::Migrator.new(:up, [migration_b], @schema_migration, 101).migrate + ActiveRecord::Migrator.new(:up, [migration_b], @schema_migration, @internal_metadata, 101).migrate assert_no_column Person, :last_name, "migration_b should have dropped the last_name column on people" - migrator = ActiveRecord::Migrator.new(:up, [migration_c], @schema_migration, 102) + migrator = ActiveRecord::Migrator.new(:up, [migration_c], @schema_migration, @internal_metadata, 102) error = assert_raises do migrator.migrate @@ -91,6 +92,7 @@ def migrate(x) class BulkAlterTableMigrationsTest < ActiveRecord::TestCase def setup + @original_test = ::BulkAlterTableMigrationsTest.new(name) @connection = Person.connection @connection.create_table(:delete_me, force: true) { |t| } Person.reset_column_information @@ -101,76 +103,38 @@ def setup Person.connection.drop_table(:delete_me) rescue nil end - def test_adding_multiple_columns - classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] - expected_query_count = { - "CockroachDBAdapter" => 8, # 5 new explicit columns, 2 for created_at/updated_at, 1 comment - }.fetch(classname) { - raise "need an expected query count for #{classname}" - } - - assert_queries(expected_query_count) do - with_bulk_change_table do |t| - t.column :name, :string - t.string :qualification, :experience - t.integer :age, default: 0 - t.date :birthdate, comment: "This is a comment" - t.timestamps null: true - end - end - - assert_equal 8, columns.size - [:name, :qualification, :experience].each { |s| assert_equal :string, column(s).type } - assert_equal "0", column(:age).default - assert_equal "This is a comment", column(:birthdate).comment - end - - def test_adding_indexes - with_bulk_change_table do |t| - t.string :username - t.string :name - t.integer :age - end - - classname = ActiveRecord::Base.connection.class.name[/[^:]*$/] - expected_query_count = { - "CockroachDBAdapter" => 2, - }.fetch(classname) { - raise "need an expected query count for #{classname}" - } - - assert_queries(expected_query_count) do - with_bulk_change_table do |t| - t.index :username, unique: true, name: :awesome_username_index - t.index [:name, :age] - end + %i( + test_adding_indexes + test_removing_index + test_adding_multiple_columns + test_changing_index + ).each do |method_name| + file, line = ::BulkAlterTableMigrationsTest.instance_method(method_name).source_location + iter = File.foreach(file) + (line - 1).times { iter.next } + indent = iter.next[/\A\s*/] + content = +"" + content << iter.next until iter.peek == indent + "end\n" + content['"PostgreSQLAdapter" =>'] = '"CockroachDBAdapter" =>' + eval(<<-RUBY, binding, __FILE__, __LINE__ + 1) + def #{method_name} + #{content} end + RUBY end private - def with_bulk_change_table - # Reset columns/indexes cache as we're changing the table - @columns = @indexes = nil - - Person.connection.change_table(:delete_me, bulk: true) do |t| - yield t - end - end - - def column(name) - columns.detect { |c| c.name == name.to_s } - end - - def columns - @columns ||= Person.connection.columns("delete_me") - end - - def index(name) - indexes.detect { |i| i.name == name.to_s } - end - def indexes - @indexes ||= Person.connection.indexes("delete_me") + %i[ + with_bulk_change_table + column + columns + index + indexes + ].each do |method| + define_method(method) do |*args, &block| + @original_test.send(method, *args, &block) end + end end end diff --git a/test/cases/relation/aost_test.rb b/test/cases/relation/aost_test.rb index 31b5fa28..d32697c5 100644 --- a/test/cases/relation/aost_test.rb +++ b/test/cases/relation/aost_test.rb @@ -50,7 +50,7 @@ def test_aost_with_multiple_queries Post.aost(time).limit(2).find_each(batch_size: 1).to_a } queries.each do - assert_match /FROM \"posts\" AS OF SYSTEM TIME '#{Regexp.quote time.iso8601}'/, _1 + assert_match %r(FROM "posts" AS OF SYSTEM TIME '#{Regexp.quote time.iso8601}'), _1 end end end diff --git a/test/cases/relation_test.rb b/test/cases/relation_test.rb index 6d2d9eda..9765688d 100644 --- a/test/cases/relation_test.rb +++ b/test/cases/relation_test.rb @@ -21,14 +21,6 @@ def test_relation_with_annotation_filters_sql_comment_delimiters post_with_annotation = Post.where(id: 1).annotate("**//foo//**") assert_includes post_with_annotation.to_sql, "= '1' /* ** //foo// ** */" end - - def test_respond_to_for_non_selected_element - post = Post.select(:title).first - assert_not_respond_to post, :body, "post should not respond_to?(:body) since invoking it raises exception" - - silence_stream($stdout) { post = Post.select("'title' as post_title").first } - assert_not_respond_to post, :title, "post should not respond_to?(:body) since invoking it raises exception" - end end end end diff --git a/test/cases/relations_test.rb b/test/cases/relations_test.rb index 888ea41f..03d7cf23 100644 --- a/test/cases/relations_test.rb +++ b/test/cases/relations_test.rb @@ -14,5 +14,16 @@ def test_finding_with_subquery_with_eager_loading_in_from assert_equal relation.to_a, Comment.select("subquery.*").from(relation).order(:id).to_a assert_equal relation.to_a, Comment.select("a.*").from(relation, :a).order(:id).to_a end + + def test_finding_with_arel_sql_order + query = Tag.order(Arel.sql("field(id, ?)", [1, 3, 2])).to_sql + assert_match(/field\(id, '1', '3', '2'\)/, query) + + query = Tag.order(Arel.sql("field(id, ?)", [])).to_sql + assert_match(/field\(id, NULL\)/, query) + + query = Tag.order(Arel.sql("field(id, ?)", nil)).to_sql + assert_match(/field\(id, NULL\)/, query) + end end end diff --git a/test/cases/schema_dumper_test.rb b/test/cases/schema_dumper_test.rb index ccecc463..47d1a9f6 100644 --- a/test/cases/schema_dumper_test.rb +++ b/test/cases/schema_dumper_test.rb @@ -11,7 +11,8 @@ class SchemaDumperTest < ActiveRecord::TestCase self.use_transactional_tests = false setup do - ActiveRecord::SchemaMigration.create_table + @schema_migration = ActiveRecord::Base.connection.schema_migration + @schema_migration.create_table end def standard_dump @@ -22,149 +23,90 @@ def perform_schema_dump dump_all_table_schema [] end - if current_adapter?(:PostgreSQLAdapter) - def test_schema_dump_with_timestamptz_datetime_format - migration, original, $stdout = nil, $stdout, StringIO.new - - with_cockroachdb_datetime_type(:timestamptz) do - migration = Class.new(ActiveRecord::Migration::Current) do - def up - create_table("timestamps") do |t| - t.datetime :this_should_remain_datetime - t.timestamptz :this_is_an_alias_of_datetime - t.column :without_time_zone, :timestamp - t.column :with_time_zone, :timestamptz - end - end - def down - drop_table("timestamps") - end - end - migration.migrate(:up) - - output = perform_schema_dump - assert output.include?('t.datetime "this_should_remain_datetime"') - assert output.include?('t.datetime "this_is_an_alias_of_datetime"') - assert output.include?('t.timestamp "without_time_zone"') - assert output.include?('t.datetime "with_time_zone"') - end - ensure - migration.migrate(:down) - $stdout = original - end + # OVERRIDE: we removed the 'deferrable' part in `assert_match` + def test_schema_dumps_unique_constraints + output = dump_table_schema("test_unique_constraints") + constraint_definitions = output.split(/\n/).grep(/t\.unique_constraint/) - def test_schema_dump_with_correct_timestamp_types_via_add_column_with_type_as_string - migration, original, $stdout = nil, $stdout, StringIO.new + assert_equal 3, constraint_definitions.size + assert_match 't.unique_constraint ["position_1"], name: "test_unique_constraints_position_1"', output + assert_match 't.unique_constraint ["position_2"], name: "test_unique_constraints_position_2"', output + assert_match 't.unique_constraint ["position_3"], name: "test_unique_constraints_position_3"', output + end - with_cockroachdb_datetime_type(:timestamptz) do - migration = Class.new(ActiveRecord::Migration[6.1]) do - def up - create_table("timestamps") + def test_schema_dump_with_timestamptz_datetime_format + migration, original, $stdout = nil, $stdout, StringIO.new - add_column :timestamps, :this_should_change_to_timestamp, "datetime" - add_column :timestamps, :this_should_stay_as_timestamp, "timestamp" - end - def down - drop_table("timestamps") + with_cockroachdb_datetime_type(:timestamptz) do + migration = Class.new(ActiveRecord::Migration::Current) do + def up + create_table("timestamps") do |t| + t.datetime :this_should_remain_datetime + t.timestamptz :this_is_an_alias_of_datetime + t.column :without_time_zone, :timestamp + t.column :with_time_zone, :timestamptz end end - migration.migrate(:up) - - output = perform_schema_dump - # Normally we'd write `t.datetime` here. But because you've changed the `datetime_type` - # to something else, `t.datetime` now means `:timestamptz`. To ensure that old columns - # are still created as a `:timestamp` we need to change what is written to the schema dump. - # - # Typically in Rails we handle this through Migration versioning (`ActiveRecord::Migration::Compatibility`) - # but that doesn't work here because the schema dumper is not aware of which migration - # a column was added in. - assert output.include?('t.timestamp "this_should_change_to_timestamp"') - assert output.include?('t.timestamp "this_should_stay_as_timestamp"') - end - ensure - migration.migrate(:down) - $stdout = original - end - - def test_timestamps_schema_dump_before_rails_7_with_timestamptz_setting - migration, original, $stdout = nil, $stdout, StringIO.new - - with_cockroachdb_datetime_type(:timestamptz) do - migration = Class.new(ActiveRecord::Migration[6.1]) do - def up - create_table("timestamps") do |t| - t.datetime :this_should_change_to_timestamp - t.timestamp :this_should_stay_as_timestamp - t.column :this_should_also_stay_as_timestamp, :timestamp - end - end - def down - drop_table("timestamps") - end + def down + drop_table("timestamps") end - migration.migrate(:up) - - output = perform_schema_dump - # Normally we'd write `t.datetime` here. But because you've changed the `datetime_type` - # to something else, `t.datetime` now means `:timestamptz`. To ensure that old columns - # are still created as a `:timestamp` we need to change what is written to the schema dump. - # - # Typically in Rails we handle this through Migration versioning (`ActiveRecord::Migration::Compatibility`) - # but that doesn't work here because the schema dumper is not aware of which migration - # a column was added in. - - assert output.include?('t.timestamp "this_should_change_to_timestamp"') - assert output.include?('t.timestamp "this_should_stay_as_timestamp"') - assert output.include?('t.timestamp "this_should_also_stay_as_timestamp"') end - ensure - migration.migrate(:down) - $stdout = original + migration.migrate(:up) + + output = perform_schema_dump + assert output.include?('t.datetime "this_should_remain_datetime"') + assert output.include?('t.datetime "this_is_an_alias_of_datetime"') + assert output.include?('t.timestamp "without_time_zone"') + assert output.include?('t.datetime "with_time_zone"') end + ensure + migration.migrate(:down) + $stdout = original + end - def test_schema_dump_with_correct_timestamp_types_via_add_column_before_rails_7_with_timestamptz_setting - migration, original, $stdout = nil, $stdout, StringIO.new + def test_schema_dump_with_correct_timestamp_types_via_add_column_with_type_as_string + migration, original, $stdout = nil, $stdout, StringIO.new - with_cockroachdb_datetime_type(:timestamptz) do - migration = Class.new(ActiveRecord::Migration[6.1]) do - def up - create_table("timestamps") + with_cockroachdb_datetime_type(:timestamptz) do + migration = Class.new(ActiveRecord::Migration[6.1]) do + def up + create_table("timestamps") - add_column :timestamps, :this_should_change_to_timestamp, :datetime - add_column :timestamps, :this_should_stay_as_timestamp, :timestamp - end - def down - drop_table("timestamps") - end + add_column :timestamps, :this_should_change_to_timestamp, "datetime" + add_column :timestamps, :this_should_stay_as_timestamp, "timestamp" + end + def down + drop_table("timestamps") end - migration.migrate(:up) - - output = perform_schema_dump - # Normally we'd write `t.datetime` here. But because you've changed the `datetime_type` - # to something else, `t.datetime` now means `:timestamptz`. To ensure that old columns - # are still created as a `:timestamp` we need to change what is written to the schema dump. - # - # Typically in Rails we handle this through Migration versioning (`ActiveRecord::Migration::Compatibility`) - # but that doesn't work here because the schema dumper is not aware of which migration - # a column was added in. - - assert output.include?('t.timestamp "this_should_change_to_timestamp"') - assert output.include?('t.timestamp "this_should_stay_as_timestamp"') end - ensure - migration.migrate(:down) - $stdout = original + migration.migrate(:up) + + output = perform_schema_dump + # Normally we'd write `t.datetime` here. But because you've changed the `datetime_type` + # to something else, `t.datetime` now means `:timestamptz`. To ensure that old columns + # are still created as a `:timestamp` we need to change what is written to the schema dump. + # + # Typically in Rails we handle this through Migration versioning (`ActiveRecord::Migration::Compatibility`) + # but that doesn't work here because the schema dumper is not aware of which migration + # a column was added in. + assert output.include?('t.timestamp "this_should_change_to_timestamp"') + assert output.include?('t.timestamp "this_should_stay_as_timestamp"') end + ensure + migration.migrate(:down) + $stdout = original + end - def test_schema_dump_when_changing_datetime_type_for_an_existing_app - original, $stdout = $stdout, StringIO.new + def test_timestamps_schema_dump_before_rails_7_with_timestamptz_setting + migration, original, $stdout = nil, $stdout, StringIO.new - migration = Class.new(ActiveRecord::Migration::Current) do + with_cockroachdb_datetime_type(:timestamptz) do + migration = Class.new(ActiveRecord::Migration[6.1]) do def up create_table("timestamps") do |t| - t.datetime :default_format - t.column :without_time_zone, :timestamp - t.column :with_time_zone, :timestamptz + t.datetime :this_should_change_to_timestamp + t.timestamp :this_should_stay_as_timestamp + t.column :this_should_also_stay_as_timestamp, :timestamp end end def down @@ -174,88 +116,159 @@ def down migration.migrate(:up) output = perform_schema_dump - assert output.include?('t.datetime "default_format"') - assert output.include?('t.datetime "without_time_zone"') - assert output.include?('t.timestamptz "with_time_zone"') - - datetime_type_was = ActiveRecord::ConnectionAdapters::CockroachDBAdapter.datetime_type - ActiveRecord::ConnectionAdapters::CockroachDBAdapter.datetime_type = :timestamptz - - output = perform_schema_dump - assert output.include?('t.timestamp "default_format"') - assert output.include?('t.timestamp "without_time_zone"') - assert output.include?('t.datetime "with_time_zone"') - ensure - ActiveRecord::ConnectionAdapters::CockroachDBAdapter.datetime_type = datetime_type_was - migration.migrate(:down) - $stdout = original + # Normally we'd write `t.datetime` here. But because you've changed the `datetime_type` + # to something else, `t.datetime` now means `:timestamptz`. To ensure that old columns + # are still created as a `:timestamp` we need to change what is written to the schema dump. + # + # Typically in Rails we handle this through Migration versioning (`ActiveRecord::Migration::Compatibility`) + # but that doesn't work here because the schema dumper is not aware of which migration + # a column was added in. + + assert output.include?('t.timestamp "this_should_change_to_timestamp"') + assert output.include?('t.timestamp "this_should_stay_as_timestamp"') + assert output.include?('t.timestamp "this_should_also_stay_as_timestamp"') end + ensure + migration.migrate(:down) + $stdout = original + end - if ActiveRecord::Base.connection.supports_check_constraints? - def test_schema_dumps_check_constraints - constraint_definition = dump_table_schema("products").split(/\n/).grep(/t.check_constraint.*products_price_check/).first.strip - if current_adapter?(:Mysql2Adapter) - assert_equal 't.check_constraint "`price` > `discounted_price`", name: "products_price_check"', constraint_definition - else - assert_equal 't.check_constraint "(price > discounted_price)", name: "products_price_check"', constraint_definition - end - end - end + def test_schema_dump_with_correct_timestamp_types_via_add_column_before_rails_7_with_timestamptz_setting + migration, original, $stdout = nil, $stdout, StringIO.new - def test_schema_dump_defaults_with_universally_supported_types - migration = Class.new(ActiveRecord::Migration::Current) do + with_cockroachdb_datetime_type(:timestamptz) do + migration = Class.new(ActiveRecord::Migration[6.1]) do def up - create_table("defaults_with_universally_supported_types") do |t| - t.string :string_with_default, default: 'Hello!' - t.date :date_with_default, default: '2014-06-05' - t.datetime :datetime_with_default, default: '2014-06-05 07:17:04' - t.time :time_with_default, default: '2000-01-01 07:17:04' - t.decimal :decimal_with_default, precision: 20, scale: 10, default: '1234567890.0123456789' - end + create_table("timestamps") + + add_column :timestamps, :this_should_change_to_timestamp, :datetime + add_column :timestamps, :this_should_stay_as_timestamp, :timestamp end def down - drop_table("defaults_with_universally_supported_types") + drop_table("timestamps") end end migration.migrate(:up) output = perform_schema_dump + # Normally we'd write `t.datetime` here. But because you've changed the `datetime_type` + # to something else, `t.datetime` now means `:timestamptz`. To ensure that old columns + # are still created as a `:timestamp` we need to change what is written to the schema dump. + # + # Typically in Rails we handle this through Migration versioning (`ActiveRecord::Migration::Compatibility`) + # but that doesn't work here because the schema dumper is not aware of which migration + # a column was added in. + + assert output.include?('t.timestamp "this_should_change_to_timestamp"') + assert output.include?('t.timestamp "this_should_stay_as_timestamp"') + end + ensure + migration.migrate(:down) + $stdout = original + end + + def test_schema_dump_when_changing_datetime_type_for_an_existing_app + original, $stdout = $stdout, StringIO.new - assert output.include?('t.string "string_with_default", default: "Hello!"') - assert output.include?('t.date "date_with_default", default: "2014-06-05"') + migration = Class.new(ActiveRecord::Migration::Current) do + def up + create_table("timestamps") do |t| + t.datetime :default_format + t.column :without_time_zone, :timestamp + t.column :with_time_zone, :timestamptz + end + end + def down + drop_table("timestamps") + end + end + migration.migrate(:up) + + output = perform_schema_dump + assert output.include?('t.datetime "default_format"') + assert output.include?('t.datetime "without_time_zone"') + assert output.include?('t.timestamptz "with_time_zone"') + + datetime_type_was = ActiveRecord::ConnectionAdapters::CockroachDBAdapter.datetime_type + ActiveRecord::ConnectionAdapters::CockroachDBAdapter.datetime_type = :timestamptz + + output = perform_schema_dump + assert output.include?('t.timestamp "default_format"') + assert output.include?('t.timestamp "without_time_zone"') + assert output.include?('t.datetime "with_time_zone"') + ensure + ActiveRecord::ConnectionAdapters::CockroachDBAdapter.datetime_type = datetime_type_was + migration.migrate(:down) + $stdout = original + end - if supports_datetime_with_precision? - assert output.include?('t.datetime "datetime_with_default", default: "2014-06-05 07:17:04"') + if ActiveRecord::Base.connection.supports_check_constraints? + def test_schema_dumps_check_constraints + constraint_definition = dump_table_schema("products").split(/\n/).grep(/t.check_constraint.*products_price_check/).first.strip + if current_adapter?(:Mysql2Adapter) + assert_equal 't.check_constraint "`price` > `discounted_price`", name: "products_price_check"', constraint_definition else - assert output.include?('t.datetime "datetime_with_default", precision: nil, default: "2014-06-05 07:17:04"') + assert_equal 't.check_constraint "(price > discounted_price)", name: "products_price_check"', constraint_definition end + end + end - assert output.include?('t.time "time_with_default", default: "2000-01-01 07:17:04"') - assert output.include?('t.decimal "decimal_with_default", precision: 20, scale: 10, default: "1234567890.0123456789"') - ensure - migration.migrate(:down) + def test_schema_dump_defaults_with_universally_supported_types + original, $stdout = $stdout, StringIO.new + + migration = Class.new(ActiveRecord::Migration::Current) do + def up + create_table("defaults_with_universally_supported_types") do |t| + t.string :string_with_default, default: 'Hello!' + t.date :date_with_default, default: '2014-06-05' + t.datetime :datetime_with_default, default: '2014-06-05 07:17:04' + t.time :time_with_default, default: '2000-01-01 07:17:04' + t.decimal :decimal_with_default, precision: 20, scale: 10, default: '1234567890.0123456789' + end + end + def down + drop_table("defaults_with_universally_supported_types") + end end + migration.migrate(:up) - if supports_text_column_with_default? - def test_schema_dump_with_text_column - migration = Class.new(ActiveRecord::Migration::Current) do - def up - create_table("text_column_with_default") do |t| - t.text :text_with_default, default: "John' Doe" - end - end - def down - drop_table("text_column_with_default") + output = perform_schema_dump + + assert output.include?('t.string "string_with_default", default: "Hello!"') + assert output.include?('t.date "date_with_default", default: "2014-06-05"') + + if supports_datetime_with_precision? + assert output.include?('t.datetime "datetime_with_default", default: "2014-06-05 07:17:04"') + else + assert output.include?('t.datetime "datetime_with_default", precision: nil, default: "2014-06-05 07:17:04"') + end + + assert output.include?('t.time "time_with_default", default: "2000-01-01 07:17:04"') + assert output.include?('t.decimal "decimal_with_default", precision: 20, scale: 10, default: "1234567890.0123456789"') + ensure + migration.migrate(:down) + $stdout = original + end + + if supports_text_column_with_default? + def test_schema_dump_with_text_column + migration = Class.new(ActiveRecord::Migration::Current) do + def up + create_table("text_column_with_default") do |t| + t.text :text_with_default, default: "John' Doe" end end - migration.migrate(:up) + def down + drop_table("text_column_with_default") + end + end + migration.migrate(:up) - output = perform_schema_dump + output = perform_schema_dump - assert output.include?('t.text "text_with_default", default: "John\' Doe"') - ensure - migration.migrate(:down) - end + assert output.include?('t.text "text_with_default", default: "John\' Doe"') + ensure + migration.migrate(:down) end end end diff --git a/test/cases/scoping/named_scoping_test.rb b/test/cases/scoping/named_scoping_test.rb deleted file mode 100644 index 166f998a..00000000 --- a/test/cases/scoping/named_scoping_test.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -require "cases/helper" -require "models/post" -require "models/topic" -require "models/comment" -require "models/reply" -require "models/author" -require "models/developer" -require "models/computer" - -module ActiveRecord - module CockroachDB - class NamedScopingTest < ActiveRecord::TestCase - fixtures :posts, :authors, :topics, :comments, :author_addresses - - def test_reserved_scope_names - klass = Class.new(ActiveRecord::Base) do - self.table_name = "topics" - - scope :approved, -> { where(approved: true) } - - class << self - public - def pub; end - - private - def pri; end - - protected - def pro; end - end - end - - subklass = Class.new(klass) - - conflicts = [ - :create, # public class method on AR::Base - :relation, # private class method on AR::Base - :new, # redefined class method on AR::Base - :all, # a default scope - :public, # some important methods on Module and Class - :protected, - :private, - :name, - :parent, - :superclass - ] - - non_conflicts = [ - :find_by_title, # dynamic finder method - :approved, # existing scope - :pub, # existing public class method - :pri, # existing private class method - :pro, # existing protected class method - :open, # a ::Kernel method - ] - - conflicts.each do |name| - e = assert_raises(ArgumentError, "scope `#{name}` should not be allowed") do - klass.class_eval { scope name, -> { where(approved: true) } } - end - assert_match(/You tried to define a scope named "#{name}" on the model/, e.message) - - e = assert_raises(ArgumentError, "scope `#{name}` should not be allowed") do - subklass.class_eval { scope name, -> { where(approved: true) } } - end - assert_match(/You tried to define a scope named "#{name}" on the model/, e.message) - end - - non_conflicts.each do |name| - assert_nothing_raised do - silence_stream($stdout) do - klass.class_eval { scope name, -> { where(approved: true) } } - end - end - - assert_nothing_raised do - subklass.class_eval { scope name, -> { where(approved: true) } } - end - end - end - end - end -end diff --git a/test/cases/tasks/cockroachdb_rake_test.rb b/test/cases/tasks/cockroachdb_rake_test.rb index 7c554420..2d7b1526 100644 --- a/test/cases/tasks/cockroachdb_rake_test.rb +++ b/test/cases/tasks/cockroachdb_rake_test.rb @@ -36,8 +36,6 @@ def test_structure_dump returns: ["accounts", "articles"] ) do ActiveRecord::Tasks::DatabaseTasks.structure_dump(config, @filename) - - read = File.read(@filename) end ensure ActiveRecord::Base.connection.execute(<<~SQL) diff --git a/test/cases/test_fixtures_test.rb b/test/cases/test_fixtures_test.rb index 29cd6c36..77fbe385 100644 --- a/test/cases/test_fixtures_test.rb +++ b/test/cases/test_fixtures_test.rb @@ -21,45 +21,6 @@ class TestFixturesTest < ActiveRecord::TestCase self.use_transactional_tests = false unless in_memory_db? - def test_doesnt_rely_on_active_support_test_case_specific_methods_with_legacy_connection_handling - old_value = ActiveRecord.legacy_connection_handling - ActiveRecord.legacy_connection_handling = true - - tmp_dir = Dir.mktmpdir - File.write(File.join(tmp_dir, "zines.yml"), <<~YML) - going_out: - title: Hello - YML - - klass = Class.new(Minitest::Test) do - include ActiveRecord::TestFixtures - - self.fixture_path = tmp_dir - - fixtures :all - - def test_run_successfully - assert_equal("Hello", Zine.first.title) - assert_equal("Hello", zines(:going_out).title) - end - end - - old_handler = ActiveRecord::Base.connection_handler - ActiveRecord::Base.connection_handler = ActiveRecord::ConnectionAdapters::ConnectionHandler.new - assert_deprecated do - ActiveRecord::Base.connection_handlers = {} - end - ActiveRecord::Base.establish_connection(:arunit) - - test_result = klass.new("test_run_successfully").run - assert_predicate(test_result, :passed?) - ensure - clean_up_legacy_connection_handlers - ActiveRecord::Base.connection_handler = old_handler - FileUtils.rm_r(tmp_dir) - ActiveRecord.legacy_connection_handling = old_value - end - def test_doesnt_rely_on_active_support_test_case_specific_methods tmp_dir = Dir.mktmpdir File.write(File.join(tmp_dir, "zines.yml"), <<~YML) @@ -70,7 +31,7 @@ def test_doesnt_rely_on_active_support_test_case_specific_methods klass = Class.new(Minitest::Test) do include ActiveRecord::TestFixtures - self.fixture_path = tmp_dir + self.fixture_paths = [tmp_dir] fixtures :all diff --git a/test/cases/unsafe_raw_sql_test.rb b/test/cases/unsafe_raw_sql_test.rb new file mode 100644 index 00000000..1183e531 --- /dev/null +++ b/test/cases/unsafe_raw_sql_test.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require "cases/helper_cockroachdb" +require "models/post" +require "models/comment" + +module CockroachDB + class UnsafeRawSqlTest < ActiveRecord::TestCase + fixtures :posts, :comments + + # OVERRIDE: We use the PostgreSQL `collation_name` for our adapter. + test "order: allows valid arguments with COLLATE" do + collation_name = "C" # <- Here is the overriden part. + + ids_expected = Post.order(Arel.sql(%Q'author_id, title COLLATE "#{collation_name}" DESC')).pluck(:id) + + ids = Post.order(["author_id", %Q'title COLLATE "#{collation_name}" DESC']).pluck(:id) + + assert_equal ids_expected, ids + end + end +end diff --git a/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapterTest.rb b/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapterTest.rb index 689ba06d..ba5c0fe6 100644 --- a/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapterTest.rb +++ b/test/excludes/ActiveRecord/ConnectionAdapters/PostgreSQLAdapterTest.rb @@ -13,4 +13,16 @@ exclude :test_serial_sequence, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_database_exists_returns_false_when_the_database_does_not_exist, "Test is reimplemented to use cockroachdb adapter" exclude :test_database_exists_returns_true_when_the_database_exists, "Test is reimplemented to use cockroachdb adapter" -exclude :test_only_reload_type_map_once_for_every_unrecognized_type, "Rewrite to replace \"silence_warnings\" by \"silence_stream\"" +exclude :test_invalid_index, "The error message differs from PostgreSQL." +exclude :test_partial_index_on_column_named_like_keyword, "CockroachDB adds parentheses around the WHERE definition." +exclude :test_only_check_for_insensitive_comparison_capability_once, "the DROP DOMAIN syntax is not supported by CRDB" + + +plpgsql_needed = "Will be testable with CRDB 23.2, introducing PL-PGSQL" +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_allowlist_of_warnings_to_ignore, plpgsql_needed +exclude :test_allowlist_of_warning_codes_to_ignore, plpgsql_needed diff --git a/test/excludes/ActiveRecord/ConnectionAdapters/SchemaCacheTest.rb b/test/excludes/ActiveRecord/ConnectionAdapters/SchemaCacheTest.rb deleted file mode 100644 index 2ce6d41a..00000000 --- a/test/excludes/ActiveRecord/ConnectionAdapters/SchemaCacheTest.rb +++ /dev/null @@ -1,3 +0,0 @@ -exclude :test_yaml_loads_5_1_dump, "The test errors because it doesn't properly reference the path to the schema dump. This is a bug in ActiveRecord, and any fix is unlikely to be backported to earlier versions of Rails. See https://github.com/rails/rails/pull/38945." -exclude :test_yaml_loads_5_1_dump_without_indexes_still_queries_for_indexes, "The test errors because it doesn't properly reference the path to the schema dump. This is a bug in ActiveRecord, and any fix is unlikely to be backported to earlier versions of Rails. See https://github.com/rails/rails/pull/38945." -exclude :test_yaml_loads_5_1_dump_without_database_version_still_queries_for_database_version, "The test errors because it doesn't properly reference the path to the schema dump. This is a bug in ActiveRecord, and any fix is unlikely to be backported to earlier versions of Rails. See https://github.com/rails/rails/pull/38945." diff --git a/test/excludes/ActiveRecord/Migration/CheckConstraintTest.rb b/test/excludes/ActiveRecord/Migration/CheckConstraintTest.rb index b8cd4b41..a4ad3566 100644 --- a/test/excludes/ActiveRecord/Migration/CheckConstraintTest.rb +++ b/test/excludes/ActiveRecord/Migration/CheckConstraintTest.rb @@ -2,3 +2,5 @@ exclude :test_remove_check_constraint, "This test is failing with transactions due to cockroachdb/cockroach#19444" exclude :test_check_constraints, "Re-implementing because some constraints are now written in parenthesis" exclude :test_add_check_constraint, "Re-implementing because some constraints are now written in parenthesis" +exclude :test_schema_dumping_with_validate_false, "Re-implementing because some constraints are now written in parenthesis" +exclude :test_schema_dumping_with_validate_true, "Re-implementing because some constraints are now written in parenthesis" diff --git a/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb b/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb index 9e802ab6..b8761bbf 100644 --- a/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb +++ b/test/excludes/ActiveRecord/Migration/CompatibilityTest.rb @@ -1 +1,8 @@ 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 } diff --git a/test/excludes/ActiveRecord/Migration/CompositeForeignKeyTest.rb b/test/excludes/ActiveRecord/Migration/CompositeForeignKeyTest.rb new file mode 100644 index 00000000..9c8957d1 --- /dev/null +++ b/test/excludes/ActiveRecord/Migration/CompositeForeignKeyTest.rb @@ -0,0 +1,2 @@ +exclude :test_add_composite_foreign_key_raises_without_options, "Updated regexp to remove quotes" +exclude :test_schema_dumping, CRDB_VALIDATE_BUG diff --git a/test/excludes/ActiveRecord/Migration/ForeignKeyTest.rb b/test/excludes/ActiveRecord/Migration/ForeignKeyTest.rb index a05a1844..fca52c1d 100644 --- a/test/excludes/ActiveRecord/Migration/ForeignKeyTest.rb +++ b/test/excludes/ActiveRecord/Migration/ForeignKeyTest.rb @@ -18,7 +18,10 @@ exclude :test_validate_foreign_key_by_name, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass." exclude :test_validate_constraint_by_name, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass." exclude :test_schema_dumping, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass." -exclude :test_schema_dumping_on_delete_and_on_update_options, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass." +exclude :test_schema_dumping_on_delete_and_on_update_options, CRDB_VALIDATE_BUG +exclude :test_schema_dumping_with_validate_false, CRDB_VALIDATE_BUG +exclude :test_schema_dumping_with_validate_true, CRDB_VALIDATE_BUG +exclude :test_schema_dumping_with_custom_fk_ignore_pattern, CRDB_VALIDATE_BUG exclude :test_add_foreign_key_is_reversible, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass." exclude :test_foreign_key_constraint_is_not_cached_incorrectly, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass." exclude :test_add_foreign_key_with_prefix, "This test fails due to the transactional nature of the test. Disabling transactions will make the test pass." diff --git a/test/excludes/ActiveRecord/Migration/IndexTest.rb b/test/excludes/ActiveRecord/Migration/IndexTest.rb deleted file mode 100644 index 0eb106bf..00000000 --- a/test/excludes/ActiveRecord/Migration/IndexTest.rb +++ /dev/null @@ -1,17 +0,0 @@ -exclude :test_index_exists_with_custom_name_checks_columns, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_named_index_exists, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_rename_index_too_long, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_add_index_works_with_long_index_names, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_index_exists_on_multiple_columns, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_add_index, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_add_index_attribute_length_limit, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_index_exists, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_remove_named_index, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_internal_index_with_name_matching_database_limit, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_rename_index, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_unique_index_exists, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_add_partial_index, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_index_symbol_names, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_add_index_with_if_not_exists_matches_exact_index, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_remove_index_with_name_which_does_not_exist_doesnt_raise_with_option, "This test runs schema changes in transactions. We run our own version with transactions disabled." -exclude :test_remove_index_which_does_not_exist_doesnt_raise_with_option, "This test runs schema changes in transactions. We run our own version with transactions disabled." diff --git a/test/excludes/ActiveRecord/Migration/InvalidOptionsTest.rb b/test/excludes/ActiveRecord/Migration/InvalidOptionsTest.rb new file mode 100644 index 00000000..fc3c89c9 --- /dev/null +++ b/test/excludes/ActiveRecord/Migration/InvalidOptionsTest.rb @@ -0,0 +1,14 @@ +module Ext + def invalid_add_column_option_exception_message(key) + default_keys = [":limit", ":precision", ":scale", ":default", ":null", ":collation", ":comment", ":primary_key", ":if_exists", ":if_not_exists"] + + # PostgreSQL specific options + default_keys.concat([":array", ":using", ":cast_as", ":as", ":type", ":enum_type", ":stored"]) + + # CockroachDB specific options + default_keys.concat([":srid", ":has_z", ":has_m", ":geographic", ":spatial_type", ":hidden"]) + + "Unknown key: :#{key}. Valid keys are: #{default_keys.join(", ")}" + end +end +prepend Ext diff --git a/test/excludes/ActiveRecord/Migration/ReferencesForeignKeyInCreateTest.rb b/test/excludes/ActiveRecord/Migration/ReferencesForeignKeyInCreateTest.rb new file mode 100644 index 00000000..261f22cb --- /dev/null +++ b/test/excludes/ActiveRecord/Migration/ReferencesForeignKeyInCreateTest.rb @@ -0,0 +1,4 @@ +no_deferrable = "CRDB does not support DEFERRABLE constraints" +exclude "test_deferrable:_:immediate_option_can_be_passed", no_deferrable +exclude "test_deferrable:_:deferred_option_can_be_passed", no_deferrable +exclude "test_deferrable_and_on_(delete|update)_option_can_be_passed", no_deferrable diff --git a/test/excludes/ActiveRecord/Migration/UniqueConstraintTest.rb b/test/excludes/ActiveRecord/Migration/UniqueConstraintTest.rb new file mode 100644 index 00000000..b2bb846c --- /dev/null +++ b/test/excludes/ActiveRecord/Migration/UniqueConstraintTest.rb @@ -0,0 +1,16 @@ +exclude :test_unique_constraints, "Removed deferrable part from constraints." + +no_deferrable = "CRDB doesn't support DEFERRABLE constraints" +exclude :test_add_unique_constraint_with_deferrable_deferred, no_deferrable +exclude :test_add_unique_constraint_with_deferrable_immediate, no_deferrable +exclude :test_added_deferrable_initially_immediate_unique_constraint, no_deferrable + +no_using_index = "CRDB doesn't support USING INDEX" +exclude :test_add_unique_constraint_with_name_and_using_index, no_using_index +exclude :test_add_unique_constraint_with_only_using_index, no_using_index + +no_remove_unique_constraint = "CRDB doesn't support " \ + "ALTER TABLE DROP CONSTRAINT. There may be an altenative, see " \ + "https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/304" +exclude :test_remove_unique_constraint, no_remove_unique_constraint +exclude :test_remove_unique_constraint_by_column, no_remove_unique_constraint diff --git a/test/excludes/ActiveRecord/MysqlDBCreateWithInvalidPermissionsTest.rb b/test/excludes/ActiveRecord/MysqlDBCreateWithInvalidPermissionsTest.rb new file mode 100644 index 00000000..5809e5b7 --- /dev/null +++ b/test/excludes/ActiveRecord/MysqlDBCreateWithInvalidPermissionsTest.rb @@ -0,0 +1 @@ +exclude :test_raises_error, "No link with MySQL" diff --git a/test/excludes/ActiveRecord/PostgresqlConnectionTest.rb b/test/excludes/ActiveRecord/PostgresqlConnectionTest.rb index 00656209..681eaa01 100644 --- a/test/excludes/ActiveRecord/PostgresqlConnectionTest.rb +++ b/test/excludes/ActiveRecord/PostgresqlConnectionTest.rb @@ -4,8 +4,10 @@ exclude :test_reset, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_release_non_existent_advisory_lock, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_table_alias_length_logs_name, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" -exclude :test_set_session_variable_true, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_statement_key_is_logged, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" -exclude :test_set_session_variable_false, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_connection_options, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_reset_with_transaction, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" +exclude :test_set_session_variable_true, "Re-implemented with a CockroachDB compatible session variable." +exclude :test_set_session_variable_false, "Re-implemented with a CockroachDB compatible session variable." +exclude :test_set_session_variable_nil, "Re-implemented with a CockroachDB compatible session variable." +exclude :test_set_session_variable_default, "Re-implemented with a CockroachDB compatible session variable." diff --git a/test/excludes/ActiveRecord/RelationTest.rb b/test/excludes/ActiveRecord/RelationTest.rb index 3f095ffd..ffd01e55 100644 --- a/test/excludes/ActiveRecord/RelationTest.rb +++ b/test/excludes/ActiveRecord/RelationTest.rb @@ -1,3 +1,2 @@ exclude :test_relation_with_annotation_filters_sql_comment_delimiters, "This test is overridden for CockroachDB because this adapter adds quotes to numeric values." exclude :test_relation_with_annotation_includes_comment_in_to_sql, "This test is overridden for CockroachDB because this adapter adds quotes to numeric values." -exclude :test_respond_to_for_non_selected_element, "Rewrite to replace \"silence_warnings\" by \"silence_stream\"" diff --git a/test/excludes/Arel/Nodes/TestNodes.rb b/test/excludes/Arel/Nodes/TestNodes.rb deleted file mode 100644 index 0c05d8d3..00000000 --- a/test/excludes/Arel/Nodes/TestNodes.rb +++ /dev/null @@ -1 +0,0 @@ -exclude :test_every_arel_nodes_have_hash_eql_eqeq_from_same_class, "Overwitten, see https://github.com/rails/rails/issues/49274" diff --git a/test/excludes/BulkAlterTableMigrationsTest.rb b/test/excludes/BulkAlterTableMigrationsTest.rb index c1093e19..b36ec7a3 100644 --- a/test/excludes/BulkAlterTableMigrationsTest.rb +++ b/test/excludes/BulkAlterTableMigrationsTest.rb @@ -1,3 +1,7 @@ -exclude :test_adding_indexes, "See test/cases/migration_test.rb for CockroachDB-specific test case" 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_adding_multiple_columns, "See test/cases/migration_test.rb for CockroachDB-specific test case" +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" diff --git a/test/excludes/CompositePrimaryKeyTest.rb b/test/excludes/CompositePrimaryKeyTest.rb deleted file mode 100644 index d18684c0..00000000 --- a/test/excludes/CompositePrimaryKeyTest.rb +++ /dev/null @@ -1 +0,0 @@ -exclude :test_primary_key_issues_warning, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" diff --git a/test/excludes/CreateOrFindByWithinTransactions.rb b/test/excludes/CreateOrFindByWithinTransactions.rb new file mode 100644 index 00000000..cd8e19a9 --- /dev/null +++ b/test/excludes/CreateOrFindByWithinTransactions.rb @@ -0,0 +1,3 @@ +ref = "See issue https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/306" +exclude :test_multiple_find_or_create_by_bang_within_transactions, ref +exclude :test_multiple_find_or_create_by_within_transactions, ref diff --git a/test/excludes/EnumTest.rb b/test/excludes/EnumTest.rb deleted file mode 100644 index bdee5806..00000000 --- a/test/excludes/EnumTest.rb +++ /dev/null @@ -1,4 +0,0 @@ -exclude "test_enum_doesn't_log_a_warning_if_opting_out_of_scopes", "Rewrite to remove \"silence_warnings\" methods" -exclude "test_enum_doesn't_log_a_warning_if_no_clashes_detected", "Rewrite to remove \"silence_warnings\" methods" -exclude "test_enum_logs_a_warning_if_auto-generated_negative_scopes_would_clash_with_other_enum_names_regardless_of_order", "Rewrite to remove \"silence_warnings\" methods" -exclude "test_enum_logs_a_warning_if_auto-generated_negative_scopes_would_clash_with_other_enum_names", "Rewrite to remove \"silence_warnings\" methods" diff --git a/test/excludes/MarshalSerializationTest.rb b/test/excludes/MarshalSerializationTest.rb index 10dfbd19..4cab699e 100644 --- a/test/excludes/MarshalSerializationTest.rb +++ b/test/excludes/MarshalSerializationTest.rb @@ -1,2 +1,4 @@ exclude :test_deserializing_rails_6_1_marshal_basic, "This uses the adapter name to find a file. We run our own version that specifies which file to use." exclude :test_deserializing_rails_6_1_marshal_with_loaded_association_cache, "This uses the adapter name to find a file. We run our own version that specifies which file to use." +exclude :test_deserializing_rails_7_1_marshal_basic, "This uses the adapter name to find a file. We run our own version that specifies which file to use." +exclude :test_deserializing_rails_7_1_marshal_with_loaded_association_cache, "This uses the adapter name to find a file. We run our own version that specifies which file to use." diff --git a/test/excludes/MultiDbMigratorTest.rb b/test/excludes/MultiDbMigratorTest.rb new file mode 100644 index 00000000..0f498111 --- /dev/null +++ b/test/excludes/MultiDbMigratorTest.rb @@ -0,0 +1 @@ +exclude :test_internal_metadata_stores_environment, "Referenced in https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/305" diff --git a/test/excludes/NamedScopingTest.rb b/test/excludes/NamedScopingTest.rb deleted file mode 100644 index 28b2d103..00000000 --- a/test/excludes/NamedScopingTest.rb +++ /dev/null @@ -1 +0,0 @@ -exclude :test_reserved_scope_names, "Rewrite to replace \"silence_warnings\" by \"silence_stream\"" diff --git a/test/excludes/PersistenceTest.rb b/test/excludes/PersistenceTest.rb index 63ebddaa..9c50f1d0 100644 --- a/test/excludes/PersistenceTest.rb +++ b/test/excludes/PersistenceTest.rb @@ -1 +1,2 @@ exclude :test_reset_column_information_resets_children, "This test fails because the column is created in the same transaction in which the test attempts to assert/operate further." +exclude :test_fills_auto_populated_columns_on_creation, "See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/308" diff --git a/test/excludes/PostgreSQLExplainTest.rb b/test/excludes/PostgreSQLExplainTest.rb index 635597ad..2e0fa100 100644 --- a/test/excludes/PostgreSQLExplainTest.rb +++ b/test/excludes/PostgreSQLExplainTest.rb @@ -1,2 +1,7 @@ exclude :test_explain_with_eager_loading, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_explain_for_one_query, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" + +no_options = "Explain options are not yet supported by this adapter. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/301" +exclude :test_explain_with_options_as_symbols, no_options +exclude :test_explain_with_options_as_strings, no_options +exclude :test_explain_options_with_eager_loading, no_options diff --git a/test/excludes/PostgresqlCaseInsensitiveTest.rb b/test/excludes/PostgresqlCaseInsensitiveTest.rb deleted file mode 100644 index cfeae9e8..00000000 --- a/test/excludes/PostgresqlCaseInsensitiveTest.rb +++ /dev/null @@ -1 +0,0 @@ -exclude :test_case_insensitiveness, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" diff --git a/test/excludes/PostgresqlCitextTest.rb b/test/excludes/PostgresqlCitextTest.rb index aa24e02d..24da1287 100644 --- a/test/excludes/PostgresqlCitextTest.rb +++ b/test/excludes/PostgresqlCitextTest.rb @@ -4,3 +4,4 @@ exclude :test_write, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_citext_enabled, "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_case_insensitiveness, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" diff --git a/test/excludes/PostgresqlExtensionMigrationTest.rb b/test/excludes/PostgresqlExtensionMigrationTest.rb index 21c08143..cde6fd09 100644 --- a/test/excludes/PostgresqlExtensionMigrationTest.rb +++ b/test/excludes/PostgresqlExtensionMigrationTest.rb @@ -1,2 +1,6 @@ exclude :test_disable_extension_migration_ignores_prefix_and_suffix, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_enable_extension_migration_ignores_prefix_and_suffix, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" +no_hstore = "Extension \"hstore\" is not yet supported by CRDB" +exclude :test_disable_extension_drops_extension_when_cascading, no_hstore +exclude :test_disable_extension_raises_when_dependent_objects_exist, no_hstore +exclude :test_enable_extension_migration_with_schema, no_hstore diff --git a/test/excludes/PostgresqlHstoreTest.rb b/test/excludes/PostgresqlHstoreTest.rb index 3d995250..b8a0a30a 100644 --- a/test/excludes/PostgresqlHstoreTest.rb +++ b/test/excludes/PostgresqlHstoreTest.rb @@ -30,7 +30,6 @@ exclude :test_parse6, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_multiline, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_nil, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" -exclude :test_parse1, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_whitespace, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_parse7, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_arrow, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" @@ -38,8 +37,6 @@ exclude :test_schema_dump_with_shorthand, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_dirty_from_user_equal, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_hstore_dirty_from_database_equal, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" -exclude :test_parse2, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" -exclude :test_gen3, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_changes_with_store_accessors, "Skipping because the test uses hstore. See https://github.com/cockroachdb/cockroach/issues/41284" exclude :test_various_null, "Skipping because the test uses hstore. See https://github.com/cockroachdb/cockroach/issues/41284" exclude :test_spaces, "Skipping because the test uses hstore. See https://github.com/cockroachdb/cockroach/issues/41284" diff --git a/test/excludes/PostgresqlVirtualColumnTest.rb b/test/excludes/PostgresqlVirtualColumnTest.rb index 338c0dd4..059cbf5e 100644 --- a/test/excludes/PostgresqlVirtualColumnTest.rb +++ b/test/excludes/PostgresqlVirtualColumnTest.rb @@ -1,2 +1,2 @@ -exclude :test_schema_dumping, "Rewrite because the virtual column scalar expression is nil." exclude :test_build_fixture_sql, "Skipping because CockroachDB cannot write directly to computed columns." +exclude :test_schema_dumping, "Replaced with local version" diff --git a/test/excludes/RelationTest.rb b/test/excludes/RelationTest.rb index f1c4bdd4..470285fd 100644 --- a/test/excludes/RelationTest.rb +++ b/test/excludes/RelationTest.rb @@ -1,2 +1,3 @@ exclude :test_finding_with_sanitized_order, "Skipping until we can triage further. See https://github.com/cockroachdb/activerecord-cockroachdb-adapter/issues/48" exclude :test_finding_with_subquery_with_eager_loading_in_from, "Overridden because test depends on ordering of results." +exclude :test_finding_with_arel_sql_order, "Result is quoted in CockroachDB." diff --git a/test/excludes/SchemaAuthorizationTest.rb b/test/excludes/SchemaAuthorizationTest.rb index 782a225e..b61f3602 100644 --- a/test/excludes/SchemaAuthorizationTest.rb +++ b/test/excludes/SchemaAuthorizationTest.rb @@ -3,10 +3,6 @@ exclude :test_sequence_schema_caching, message exclude :test_session_auth=, message exclude :test_schema_invisible, message -exclude :test_schema_invisible, message -exclude :test_tables_in_current_schemas, message exclude :test_tables_in_current_schemas, message exclude :test_auth_with_bind, message -exclude :test_auth_with_bind, message -exclude :test_setting_auth_clears_stmt_cache, message exclude :test_setting_auth_clears_stmt_cache, message diff --git a/test/excludes/SchemaDumperTest.rb b/test/excludes/SchemaDumperTest.rb index 21219b4b..7d3f0f7b 100644 --- a/test/excludes/SchemaDumperTest.rb +++ b/test/excludes/SchemaDumperTest.rb @@ -10,3 +10,4 @@ exclude :test_schema_dump_with_correct_timestamp_types_via_add_column_before_rails_7_with_timestamptz_setting, "Re-implementing ourselves because we need CockroachDB specific methods." exclude :test_schema_dump_when_changing_datetime_type_for_an_existing_app, "Re-implementing ourselves because we need CockroachDB specific methods." exclude :test_schema_dumps_check_constraints, "Re-implementing because some constraints are now written in parenthesis" +exclude :test_schema_dumps_unique_constraints, "Re-implementing because DEFERRABLE is not supported by CockroachDB" diff --git a/test/excludes/SchemaForeignKeyTest.rb b/test/excludes/SchemaForeignKeyTest.rb new file mode 100644 index 00000000..95f994f9 --- /dev/null +++ b/test/excludes/SchemaForeignKeyTest.rb @@ -0,0 +1 @@ +exclude :test_dump_foreign_key_targeting_different_schema, CRDB_VALIDATE_BUG diff --git a/test/excludes/TestFixturesTest.rb b/test/excludes/TestFixturesTest.rb index 5e5f0896..d0ecf6d2 100644 --- a/test/excludes/TestFixturesTest.rb +++ b/test/excludes/TestFixturesTest.rb @@ -1,2 +1 @@ -exclude :test_doesnt_rely_on_active_support_test_case_specific_methods_with_legacy_connection_handling, "This test is replaced with a version in our test suite." exclude :test_doesnt_rely_on_active_support_test_case_specific_methods, "This test is replaced with a version in our test suite." diff --git a/test/excludes/TransactionInstrumentationTest.rb b/test/excludes/TransactionInstrumentationTest.rb new file mode 100644 index 00000000..fb7523dc --- /dev/null +++ b/test/excludes/TransactionInstrumentationTest.rb @@ -0,0 +1 @@ +exclude :test_transaction_instrumentation_with_restart_parent_transaction_on_rollback, "CRDB doesn't support ROLLBACK AND CHAIN" diff --git a/test/excludes/UnsafeRawSqlTest.rb b/test/excludes/UnsafeRawSqlTest.rb index 4a0f962e..2efa929f 100644 --- a/test/excludes/UnsafeRawSqlTest.rb +++ b/test/excludes/UnsafeRawSqlTest.rb @@ -1 +1,2 @@ exclude "test_order:_allows_NULLS_FIRST_and_NULLS_LAST_too", "This functionality is not yet implemented, see https://github.com/cockroachdb/cockroach/issues/6224 for more details" +exclude "test_order:_allows_valid_arguments_with_COLLATE", "Replaced for correct collation name" diff --git a/test/schema/cockroachdb_specific_schema.rb b/test/schema/cockroachdb_specific_schema.rb index 57dc1f71..bbbf2b76 100644 --- a/test/schema/cockroachdb_specific_schema.rb +++ b/test/schema/cockroachdb_specific_schema.rb @@ -5,11 +5,20 @@ # with an explanation of why they don't work. ActiveRecord::Schema.define do - enable_extension!("uuid-ossp", ActiveRecord::Base.connection) - enable_extension!("pgcrypto", ActiveRecord::Base.connection) if ActiveRecord::Base.connection.supports_pgcrypto_uuid? + ActiveRecord::TestCase.enable_extension!("uuid-ossp", ActiveRecord::Base.connection) + ActiveRecord::TestCase.enable_extension!("pgcrypto", ActiveRecord::Base.connection) if ActiveRecord::Base.connection.supports_pgcrypto_uuid? uuid_default = connection.supports_pgcrypto_uuid? ? {} : { default: "uuid_generate_v4()" } + create_table :chat_messages, id: :uuid, force: true, **uuid_default do |t| + t.text :content + end + + create_table :chat_messages_custom_pk, id: false, force: true do |t| + t.uuid :message_id, primary_key: true, default: "uuid_generate_v4()" + t.text :content + end + create_table :uuid_parents, id: :uuid, force: true, **uuid_default do |t| t.string :name end @@ -20,10 +29,15 @@ end create_table :defaults, force: true do |t| + t.virtual :virtual_stored_number, type: :integer, as: "random_number * 10", stored: true if supports_virtual_columns? + t.integer :random_number, default: -> { "random() * 100" } + t.string :ruby_on_rails, default: -> { "concat('Ruby ', 'on ', 'Rails')" } t.date :modified_date, default: -> { "CURRENT_DATE" } t.date :modified_date_function, default: -> { "now()" } t.date :fixed_date, default: "2004-01-01" t.datetime :modified_time, default: -> { "CURRENT_TIMESTAMP" } + t.datetime :modified_time_without_precision, precision: nil, default: -> { "CURRENT_TIMESTAMP" } + t.datetime :modified_time_with_precision_0, precision: 0, default: -> { "CURRENT_TIMESTAMP" } t.datetime :modified_time_function, default: -> { "now()" } t.datetime :fixed_time, default: "2004-01-01 00:00:00.000000-00" t.timestamptz :fixed_time_with_time_zone, default: "2004-01-01 01:00:00+1" @@ -36,6 +50,15 @@ " end + if supports_identity_columns? + drop_table "postgresql_identity_table", if_exists: true + execute <<~SQL + create table postgresql_identity_table ( + id INT PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY + ) + SQL + end + create_table :postgresql_times, force: true do |t| t.interval :time_interval t.interval :scaled_time_interval, precision: 6 @@ -138,6 +161,23 @@ t.string :subject end + # CockroachDB does not support exclusion constraints. + # + # create_table :test_exclusion_constraints, force: true do |t| + # ... + # end + + create_table :test_unique_constraints, force: true do |t| + t.integer :position_1 + t.integer :position_2 + t.integer :position_3 + + # CockroachDB does not support deferrable, hence these three lines have been simplified. + t.unique_constraint :position_1, name: "test_unique_constraints_position_1" + t.unique_constraint :position_2, name: "test_unique_constraints_position_2" + t.unique_constraint :position_3, name: "test_unique_constraints_position_3" + end + if supports_partitioned_indexes? create_table(:measurements, id: false, force: true, options: "PARTITION BY LIST (city_id)") do |t| t.string :city_id, null: false @@ -152,6 +192,8 @@ options: "PARTITION OF measurements FOR VALUES IN (2)") end + add_index(:companies, [:firm_id, :type], name: "company_include_index", include: [:name, :account_id]) + create_table :buildings, force: true do |t| t.st_point :coordinates, srid: 3857 t.st_point :latlon, srid: 4326, geographic: true diff --git a/test/support/rake_helpers.rb b/test/support/rake_helpers.rb index 07a82807..e73c1b8e 100644 --- a/test/support/rake_helpers.rb +++ b/test/support/rake_helpers.rb @@ -25,13 +25,13 @@ def test_files end def all_test_files - activerecord_test_files = Dir. - glob("#{ARTest::CockroachDB.root_activerecord}/test/cases/**/*_test.rb"). - reject { _1.match?(%r(/adapters/(?:mysql2|sqlite3)/)) }. - prepend(COCKROACHDB_TEST_HELPER) + activerecord_test_files = + FileList["#{ARTest::CockroachDB.root_activerecord}/test/cases/**/*_test.rb"]. + reject { _1.include?("/adapters/") || _1.include?("/encryption/performance") } + + FileList["#{ARTest::CockroachDB.root_activerecord}/test/cases/adapters/postgresql/**/*_test.rb"] - cockroachdb_test_files = Dir.glob('test/cases/**/*_test.rb') + cockroachdb_test_files = FileList['test/cases/**/*_test.rb'] - activerecord_test_files + cockroachdb_test_files + FileList[COCKROACHDB_TEST_HELPER] + activerecord_test_files + cockroachdb_test_files end end