This commit addresses CI failure of Active Record isolation tests.
https://buildkite.com/rails/rails/builds/79936
Follow up #41392
* Steps to reproduce
```ruby
$ cd activerecord
$ bin/test test/cases/associations/belongs_to_associations_test.rb test/cases/associations/has_many_associations_test.rb test/cases/associations/has_many_through_disable_joins_associations_test.rb test/cases/associations/has_one_associations_test.rb test/cases/connection_adapters/schema_cache_test.rb test/cases/inheritance_test.rb test/cases/migration_test.rb test/cases/store_test.rb test/cases/strict_loading_test.rb
```
Since the entire output is too long, here is the minimum case fixed by
this commit.
```ruby
$ cd activerecord
$ bin/test test/cases/associations/belongs_to_associations_test.rb:38
Using sqlite3
Run options: --seed 34180
E
Error:
BelongsToAssociationsTest#test_belongs_to:
NameError: Rails couldn't find a valid model for Club association. Please provide the :class_name option on the association declaration. If :class_name is already provided, make sure it's an ActiveRecord::Base subclass.
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/reflection.rb:431:in `rescue in compute_class'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/reflection.rb:422:in `compute_class'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/reflection.rb:372:in `klass'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/reflection.rb:722:in `association_primary_key'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/reflection.rb:727:in `join_primary_key'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_row.rb:150:in `block in resolve_sti_reflections'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_row.rb:138:in `each_value'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_row.rb:138:in `resolve_sti_reflections'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_row.rb:92:in `fill_row_model_attributes'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_row.rb:70:in `initialize'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_rows.rb:36:in `new'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_rows.rb:36:in `block in build_table_rows_from'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_rows.rb:35:in `each'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_rows.rb:35:in `map'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_rows.rb:35:in `build_table_rows_from'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixture_set/table_rows.rb:18:in `initialize'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:692:in `new'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:692:in `table_rows'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:633:in `block (2 levels) in insert'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:632:in `each'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:632:in `block in insert'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:629:in `each'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:629:in `insert'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:615:in `read_and_insert'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:567:in `create_fixtures'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/test_fixtures.rb:268:in `load_fixtures'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/test_fixtures.rb:122:in `setup_fixtures'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/test_fixtures.rb:10:in `before_setup'
bin/test test/cases/associations/belongs_to_associations_test.rb:38
Finished in 0.083668s, 11.9520 runs/s, 0.0000 assertions/s.
1 runs, 0 assertions, 0 failures, 1 errors, 0 skips
$
```
When an engine's migration is installed in a rails application, a
comment is inserted after any magic comments indicating the migration's
source. However, the current implementation does not take into
account whether there is an empty line after magic comments, and the
generated migration will not pass rubocop's
Layout/EmptyLineAfterMagicComment even if the engine's migration did.
This commit changes the implementation to insert the new comment after
a newline occuring after magic comments, if it exists.
Example Engine Migration:
```ruby
# frozen_string_literal: true
# coding: ISO-8859-15
class CurrenciesHaveSymbols < ActiveRecord::Migration::Current
end
```
Before change:
```ruby
# frozen_string_literal: true
# coding: ISO-8859-15
# This migration comes from bukkits (originally 1)
class CurrenciesHaveSymbols < ActiveRecord::Migration::Current
end
```
After change:
```ruby
# frozen_string_literal: true
# coding: ISO-8859-15
# This migration comes from bukkits (originally 1)
class CurrenciesHaveSymbols < ActiveRecord::Migration::Current
end
```
This allows you to specify an explicit order that you'd like records
returned in based on a SQL expression. By default, this will be accomplished
using a case statement, as in:
```ruby
Post.in_order_of(:id, [3, 5, 1])
```
will generate the SQL:
```sql
SELECT "posts".* FROM "posts" ORDER BY CASE "posts"."id" WHEN 3 THEN 1 WHEN 5 THEN 2 WHEN 1 THEN 3 ELSE 4 END ASC
```
However, because this functionality is built into MySQL in the form of the
`FIELD` function, that connection adapter will generate the following SQL
instead:
```sql
SELECT "posts".* FROM "posts" ORDER BY FIELD("posts"."id", 1, 5, 3) DESC
```
*Kevin Newton*
The language in the docs is clearer without saying MySQL is stupid. The
test doesn't need stupid in the value to be used - an incorrect value is
incorrect.
This comment was correct when it was added in 65843e1. In that commit, ActiveRecord::AssociationScope#scope applies the nearest association's scope, and #add_constraints skips it.
c8d8899 made the comment incorrect. It modified #scope not to apply the nearest association's scope and #add_constraints to apply scopes for the entire association chain.
Managed to reproduce CI failure at https://buildkite.com/rails/rails/builds/79496#0c03f856-9be1-4ca0-88c9-e1df21ae0a07
This commitaddresses the following errors by loading :author_addresses fixture because
`:authors` has a foreign key to `:author_addresses`.
* sqlite3 adapter
```ruby
$ bin/test test/cases/adapters/sqlite3/explain_test.rb test/cases/fixtures_test.rb -n "/^(?:SQLite3ExplainTest#(?:test_explain_with_eager_loading)|FixturesWithForeignKeyViolationsTest#(?:test_does_not_raise_if_no_fk_violations))$/" --seed 2529
Using sqlite3
Run options: -n "/^(?:SQLite3ExplainTest#(?:test_explain_with_eager_loading)|FixturesWithForeignKeyViolationsTest#(?:test_does_not_raise_if_no_fk_violations))$/" --seed 2529
.E
Error:
FixturesWithForeignKeyViolationsTest#test_does_not_raise_if_no_fk_violations:
RuntimeError: Foreign key violations found in your fixture data. Ensure you aren't referring to labels that don't exist on associations.
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:641:in `block in insert'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:629:in `each'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:629:in `insert'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:615:in `read_and_insert'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:567:in `create_fixtures'
/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:844:in `block (2 levels) in test_does_not_raise_if_no_fk_violations'
/home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:34:in `assert_nothing_raised'
/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:843:in `block in test_does_not_raise_if_no_fk_violations'
/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:857:in `with_verify_foreign_keys_for_fixtures'
/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:842:in `test_does_not_raise_if_no_fk_violations'
bin/test test/cases/fixtures_test.rb:835
Finished in 0.045767s, 43.6991 runs/s, 174.7966 assertions/s.
2 runs, 8 assertions, 0 failures, 1 errors, 0 skips
$
```
* postgresql adapter
```ruby
$ ARCONN=postgresql bin/test test/cases/adapters/postgresql/explain_test.rb test/cases/fixtures_test.rb -n "/^(?:PostgreSQLExplainTest#(?:test_explain_with_eager_loading)|FixturesWithForeignKeyViolationsTest#(?:test_does_not_raise_if_no_fk_violations))$/" --seed 16926
Using postgresql
Run options: -n "/^(?:PostgreSQLExplainTest#(?:test_explain_with_eager_loading)|FixturesWithForeignKeyViolationsTest#(?:test_does_not_raise_if_no_fk_violations))$/" --seed 16926
.E
Error:
FixturesWithForeignKeyViolationsTest#test_does_not_raise_if_no_fk_violations:
RuntimeError: Foreign key violations found in your fixture data. Ensure you aren't referring to labels that don't exist on associations.
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:641:in `block in insert'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:629:in `each'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:629:in `insert'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:615:in `read_and_insert'
/home/yahonda/src/github.com/rails/rails/activerecord/lib/active_record/fixtures.rb:567:in `create_fixtures'
/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:844:in `block (2 levels) in test_does_not_raise_if_no_fk_violations'
/home/yahonda/src/github.com/rails/rails/activesupport/lib/active_support/testing/assertions.rb:34:in `assert_nothing_raised'
/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:843:in `block in test_does_not_raise_if_no_fk_violations'
/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:857:in `with_verify_foreign_keys_for_fixtures'
/home/yahonda/src/github.com/rails/rails/activerecord/test/cases/fixtures_test.rb:842:in `test_does_not_raise_if_no_fk_violations'
bin/test test/cases/fixtures_test.rb:835
Finished in 0.121193s, 16.5027 runs/s, 49.5080 assertions/s.
2 runs, 6 assertions, 0 failures, 1 errors, 0 skips
$
```
* It does not reproduce with mysql2 adapter because MySQL does not have
features to validate existing foreign keys, but added
:author_addresses in Mysql2ExplainTest for consistency.
Follow up #42674
Permit checking whether a belongs_to association has been pointed to a new target record in the previous save and whether it will point to a new target record in the next save.
post.category # => #<Category id: 1, name: "Ruby">
post.category = Category.second # => #<Category id: 2, name: "Programming">
post.category_changed? # => true
post.category_previously_changed? # => false
post.save!
post.category_changed? # => false
post.category_previously_changed? # => true
Dumping the schema is on by default for all databases in an application. To turn it off for a
specific database use the `schema_dump` option:
```yaml
# config/database.yml
production:
schema_dump: false
```
Co-authored-by: Luis Vasconcellos <vasconcelloslf@gmail.com>
When you're using a custom primary key on a belongs_to and you're trying to load that value through the association shorthand in a fixture, you end up getting the primary key of the table and not the primary key specified in the join. This makes sense to keep as the behavior because it's super fast (just hashing the name of the fixture), but it's still surprising so we should want the developer that it's not possible to do what they want.
Sometimes when we are preloading associations on records, we already have
the association objects loaded in-memory. But the Preloader will go to the
database to fetch them anyways because there is no way to tell it about
these loaded objects. This PR adds a new `available_records` argument to
supply the Preloader with a set of in-memory records that it can use to
fill associations without making a database query.
`available_records` is an array of ActiveRecord::Base objects. Mixed
models are supported. Preloader will use these records to try to fill the
requested associations before querying the database. This can save database
queries by reusing in-memory objects.
The optimization is only applied to single associations (i.e. :belongs_to,
:has_one) as we must query the database to do an exhaustive lookup for
collections (i.e. :has_many). It also requires the query to have no scopes
as scopes can filter associations even if they are already in memory.
```ruby
comment = Comment.last
post = Post.find_by(id: comment.post_id)
all_authors = Author.all.to_a
Preloader.new([comment], [:post, :author]).call
Preloader.new([comment], [:post, :author], available_records: [post, all_authors]).call
```
Co-Authored-By: John Hawthorn <john@hawthorn.email>
A good part of the complexity was to support the HashLookupTypeMap
subclass that's only used by the Postgres adapter.
In the end they have a similar-ish interface but this inheritance
doesn't help much.
Worse, adapters using `TypeMap` doesn't need extra arguments. By
skipping this unused feature, we can drastically reduce the memory
footprint, as this feature imposed a 320B per entry overhead.
Each type map can use a non trivial amount of memory (over 10KiB
in our app).
Currently each connection build its own type map from scratch, but
except for postgresql which has extension types, all connections
end up with the same maps.
So the more connections you have the more memory it wastes.
By defining the type map statically for MySQL and SQLite3 connections
we save some memory, share caches, and allow that memory to be handled
by Copy on Write for forking setups.
Ref: https://github.com/rails/rails/pull/42355
The justification for `partial_inserts` back in 2012
(144e8691cbfb8bba77f18cfe68d5e7fd48887f5e) was:
> This is more efficient, and also means that it will be safe to remove
> database columns without getting subsequent errors in running app processes
> (so long as the code in those processes doesn't contain any references to the
> removed column).
But since then `ignored_columns` is a much more reliable way to safely remove a
column, and I doubt the reduced query size really help much.
Additionally, `partial_inserts` prevent removing the default value of a column
in a safe way.
This reverts commit 1cdee90d385399083596c22343cc46f51283aca3, reversing
changes made to c3a1bfe187d594ed2c2348cc0ae6c5c894368b8e.
We are reverting this because it caused a change in behavior and we need
to discuss how to address that behavior change and find a work around
that doesn't break existing applications.
https://github.com/rails/rails/pull/42601 fixed clearing the inverse relation,
but it didn't account for collection associations.
For these, just assigning `nil` isn't possible because we need the record to
remove it from the collection.
So this PR introduce an explicit method for this purpose rather than
reuse `inversed_from(nil)`.
When writing fixtures, it's currently possible to define associations that don't exist, even if a foreign key exists. For example:
```yml
george:
name: "Curious George"
pirate: redbeard
blackbeard:
name: "Blackbeard"
```
When the fixtures are created, `parrots(:george).pirate` will be nil, but it's not immediately clear why. This can make it hard to debug tests and can give false confidence in passing ones.
This can happen because Rails [disables referential integrity](f263530bf7/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb (L407)) when inserting fixtures. This makes the fixtures algorithm much simpler - it can just create the fixtures in alphabetical order and assume that the other side of a foreign key constraint will *eventually* be added.
Ideally we would check foreign keys once all fixtures have been loaded, so that we can be sure that the foreign key constraints were met. This PR introduces that. To enable it:
```ruby
config.active_record.verify_foreign_keys_for_fixtures = true
```
I'm proposing we enable this in 7.0 for new apps and have added it to new framework defaults. When run against our app, it found 3 fixture files with unmet FK constraints - turns out all those fixtures weren't being used and were safe to delete.
As of Ruby 2.7 DidYouMean is included as a default gem, so there is no
need to check if DidYouMean is defined in the test suite. We still need
to check if the DidYouMean modules are defined in the actual code, as
someone might run Rails with DidYouMean disabled by using the
`--disable-did_you_mean` flag. This is ussually done for performance
reasons.
This commit also includes some of the changes made by Yuki in:
https://github.com/rails/rails/pull/39555
These changes include replacing Jaro with the more accurate
SpellChecker, and using DidYouMean::Correctable for simplere
corrections.
The DidYouMean::SpellChecker does have a treshold for corrections.
If there is not enough similarity it might not return a suggestion.
To stop the tests from failing some test data had to be changed.
For example, `non_existent` does not meet the treshold for `hello`, but
`ello` does:
DidYouMean::SpellChecker.new(dictionary: %w[hello]).correct('non_existent')
=> []
DidYouMean::SpellChecker.new(dictionary: %w[hello]).correct('ello')
=> ["hello"]
The treshold makes sense for spelling errors. But maybe we should add a
different SpellChecker that helps to get a suggestion even if there is
little overlap. For example for when a model only has 2 attributes
(title and body), it's helpful to get a suggestion for `name`
Co-Authored-By: Yuki Nishijima <yk.nishijima@gmail.com>
Given an author that "has one" post, and setting the post's author to nil,
the author will still have a cached association to the post. Updating any
attribute of the author would erroneously restore the post's author. This
commit resets the association, preventing that behavior.
In #42604 we added functionality that would sort the migrations by
timestamp so that if there were 2 migrations in 2 databases that were
dependent on one another they would be run in timestamp order rather
than in cluster order.
While debugging another issue I realized that if the timestamps are the
same that only one of the migrations will get run. This fixes the issue
by collecting all the db configs for a version so that we get the entire
list.
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
- Mention explicitly in the docs that `create_of_find_by` requires a DB
constraint.
Closes#36027
Co-authored-by: Eileen M. Uchitelle <eileencodes@users.noreply.github.com>
💇♀️
Fix specs
Update activerecord/lib/active_record/relation.rb
Co-authored-by: Eugene Kenny <elkenny@gmail.com>
Update activerecord/lib/active_record/relation.rb
Co-authored-by: Viktar Basharymau <6alliapumob@gmail.com>
Use class attribute instead of mattr_accessor
Add destroy all test case around value returned
add Rails.application.config.active_record.destroy_all_in_batches too new defaults framework
Reset Active Record Relation
Document ActiveRecord::Relation#destroy_all
Add changelog entry and update docs
💇♀️
Update method signature and argument docs
Apply suggestions from code review
Co-authored-by: Viktar Basharymau <6alliapumob@gmail.com>
Co-authored-by: Alberto Almagro <albertoalmagro@gmail.com>
Applications can set their migrations to ignore exceptions raised when adding a foreign key that already exists or when removing a foreign key that does not exist.
Add test cases
💇♀️
Previously if there were 2 migrations in one db and 1 migration in the
other db all the migrations for db one would run and then all migrations
for db two would run. If a migration in one database depended on a
migration in another database then it could fail. This is probably
pretty rare, however in a multi-db application that's moving tables from
one db to another, running them out of order could result in a migration
error.
In this this change we collect all the versions for each migration and
the corresponding db_config so we can run them in the order they are
created rather than per-db.
Closes#41664
Related #41538
Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
Co-authored-by: Kiril Dokh <dsounded@gmail.com>
This reverts commit 951deecc52f191ca85bd5c0416b382f4852c6f72.
This change prevents applications from testing replicas and would
require explicitly setting `prevent_writes` when connecting to reading
roles in `connected_to`. For now we'll revert this until there's a
longer term fix in place
💇♀️
Set precision 6 by detauls for datime columns when using sqlite3_adapter
Set precision 6 by detauls for datime columns when using postgresql_adapter
Update activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
Co-authored-by: Zachary Scott <zzakscott@gmail.com>
Update tests precision
Add datetime precision by using the migration compatibility layer
Add default precision when adding new column for a table
Fix compatibility tests
Add changelog entry
Update postgres test cases
Custom type does not support precision
Typo ⌨️
Fix specific mysql2 schema
💇♀️
When a model has an STI enum type, the type wasn't properly applied when
disable joins was set to true. In this case we need to apply the scope
from the association in `add_constraints` so that `type` is included in
the query. Otherwise in a `has_one :through` with `type` all records
will be returned because we're not filtering on type.
Long term I think this might be in the wrong place and that we want to
do this in a new definition of `target_scope` but that's a future
refactoring that needs to be done.
Strict loading mode `:n_plus_one_only` is only
supported on single records, not associations or
models. Using an ivar instead of class_attribute
ensures that this cannot be set globally. This
fixes a bug where setting `strict_loading_mode`
caused errors to be silent when
`strict_loading_by_default` is true.
Fixes#42576
Co-Authored-By: John Hawthorn <john@hawthorn.email>
