From 9506aa79785e7e945ae86bc6579ce7330e38635e Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Wed, 24 Jan 2024 14:48:35 -0300 Subject: [PATCH] feat: Support Rails 7.1 This PR includes loads of small changes that won't be mentionned here to adapt to the new Rails code. Loads of tests ignored tests have been added back as they were not error prone anymore. There are likely more of these. We removed the deprecated `configure_connection` method override. It is now unnecessary as CockroachDB supports `SET TIMEZONE <...>`. We dropped support for CockroachDB version before 22.X.X. We removed a lot of copy-pasted code in the test thanks to new file separation in Rails 7.1 test configuration. Signed-off-by: Ulysse Buonomo --- .github/workflows/ci.yml | 10 +- Gemfile | 7 +- activerecord-cockroachdb-adapter.gemspec | 2 +- .../connection_adapters/cockroachdb/column.rb | 10 +- .../cockroachdb/column_methods.rb | 8 + .../cockroachdb/database_statements.rb | 83 ---- .../cockroachdb/referential_integrity.rb | 18 +- .../cockroachdb/schema_statements.rb | 43 +- .../connection_adapters/cockroachdb/type.rb | 5 +- .../cockroachdb_adapter.rb | 216 ++++------ .../relation/query_methods_ext.rb | 2 +- .../adapters/postgresql/connection_test.rb | 42 ++ .../adapters/postgresql/interval_test.rb | 9 +- .../postgresql/postgresql_adapter_test.rb | 49 ++- .../postgresql/virtual_column_test.rb | 39 +- test/cases/arel/nodes_test.rb | 40 -- .../connection_adapters/schema_cache_test.rb | 74 ---- test/cases/connection_adapters/type_test.rb | 2 + test/cases/enum_test.rb | 96 ----- test/cases/fixtures_test.rb | 11 +- test/cases/helper.rb | 276 ------------- test/cases/helper_cockroachdb.rb | 59 +++ test/cases/migration/check_constraint_test.rb | 17 + test/cases/migration/foreign_key_test.rb | 37 +- test/cases/migration/index_test.rb | 233 ----------- .../cases/migration/unique_constraint_test.rb | 51 +++ test/cases/migration_test.rb | 104 ++--- test/cases/relation/aost_test.rb | 2 +- test/cases/relation_test.rb | 8 - test/cases/relations_test.rb | 11 + test/cases/schema_dumper_test.rb | 385 +++++++++--------- test/cases/scoping/named_scoping_test.rb | 85 ---- test/cases/tasks/cockroachdb_rake_test.rb | 2 - test/cases/test_fixtures_test.rb | 41 +- test/cases/unsafe_raw_sql_test.rb | 22 + .../PostgreSQLAdapterTest.rb | 14 +- .../ConnectionAdapters/SchemaCacheTest.rb | 3 - .../Migration/CheckConstraintTest.rb | 2 + .../Migration/CompatibilityTest.rb | 7 + .../Migration/CompositeForeignKeyTest.rb | 2 + .../ActiveRecord/Migration/ForeignKeyTest.rb | 5 +- .../ActiveRecord/Migration/IndexTest.rb | 17 - .../Migration/InvalidOptionsTest.rb | 14 + .../ReferencesForeignKeyInCreateTest.rb | 4 + .../Migration/UniqueConstraintTest.rb | 16 + ...MysqlDBCreateWithInvalidPermissionsTest.rb | 1 + .../ActiveRecord/PostgresqlConnectionTest.rb | 6 +- test/excludes/ActiveRecord/RelationTest.rb | 1 - test/excludes/Arel/Nodes/TestNodes.rb | 1 - test/excludes/BulkAlterTableMigrationsTest.rb | 8 +- test/excludes/CompositePrimaryKeyTest.rb | 1 - .../CreateOrFindByWithinTransactions.rb | 3 + test/excludes/EnumTest.rb | 4 - test/excludes/MarshalSerializationTest.rb | 2 + test/excludes/MultiDbMigratorTest.rb | 1 + test/excludes/NamedScopingTest.rb | 1 - test/excludes/PersistenceTest.rb | 1 + test/excludes/PostgreSQLExplainTest.rb | 5 + .../excludes/PostgresqlCaseInsensitiveTest.rb | 1 - test/excludes/PostgresqlCitextTest.rb | 1 + .../PostgresqlExtensionMigrationTest.rb | 4 + test/excludes/PostgresqlHstoreTest.rb | 3 - test/excludes/PostgresqlVirtualColumnTest.rb | 2 +- test/excludes/RelationTest.rb | 1 + test/excludes/SchemaAuthorizationTest.rb | 4 - test/excludes/SchemaDumperTest.rb | 1 + test/excludes/SchemaForeignKeyTest.rb | 1 + test/excludes/TestFixturesTest.rb | 1 - .../TransactionInstrumentationTest.rb | 1 + test/excludes/UnsafeRawSqlTest.rb | 1 + test/schema/cockroachdb_specific_schema.rb | 46 ++- test/support/rake_helpers.rb | 12 +- 72 files changed, 823 insertions(+), 1474 deletions(-) create mode 100644 test/cases/adapters/postgresql/connection_test.rb delete mode 100644 test/cases/arel/nodes_test.rb delete mode 100644 test/cases/connection_adapters/schema_cache_test.rb delete mode 100644 test/cases/enum_test.rb delete mode 100644 test/cases/helper.rb delete mode 100644 test/cases/migration/index_test.rb create mode 100644 test/cases/migration/unique_constraint_test.rb delete mode 100644 test/cases/scoping/named_scoping_test.rb create mode 100644 test/cases/unsafe_raw_sql_test.rb delete mode 100644 test/excludes/ActiveRecord/ConnectionAdapters/SchemaCacheTest.rb create mode 100644 test/excludes/ActiveRecord/Migration/CompositeForeignKeyTest.rb delete mode 100644 test/excludes/ActiveRecord/Migration/IndexTest.rb create mode 100644 test/excludes/ActiveRecord/Migration/InvalidOptionsTest.rb create mode 100644 test/excludes/ActiveRecord/Migration/ReferencesForeignKeyInCreateTest.rb create mode 100644 test/excludes/ActiveRecord/Migration/UniqueConstraintTest.rb create mode 100644 test/excludes/ActiveRecord/MysqlDBCreateWithInvalidPermissionsTest.rb delete mode 100644 test/excludes/Arel/Nodes/TestNodes.rb delete mode 100644 test/excludes/CompositePrimaryKeyTest.rb create mode 100644 test/excludes/CreateOrFindByWithinTransactions.rb delete mode 100644 test/excludes/EnumTest.rb create mode 100644 test/excludes/MultiDbMigratorTest.rb delete mode 100644 test/excludes/NamedScopingTest.rb delete mode 100644 test/excludes/PostgresqlCaseInsensitiveTest.rb create mode 100644 test/excludes/SchemaForeignKeyTest.rb create mode 100644 test/excludes/TransactionInstrumentationTest.rb 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