| Commit message (Collapse) | Author | Age | Files | Lines |
|\
| |
| | |
Warn if we can't read the yaml to create database tasks
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
For multiple databases we attempt to generate the tasks by reading the
database.yml before the Rails application is booted. This means that we
need to strip out ERB since it could be reading Rails configs.
In some cases like https://github.com/rails/rails/issues/36540 the ERB
is too complex and we can't overwrite with the DummyCompilier we used in
https://github.com/rails/rails/pull/35497. For the complex causes we
simply issue a warning that says we couldn't infer the database tasks
from the database.yml.
While working on this I decided to update the code to only load the
database.yml once initially so that we avoid having to issue the same
warning multiple times. Note that this had no performance impact in my
testing and is merely for not having to save the error off somewhere.
Also this feels cleaner.
Note that this will not break running tasks that exist, it will just
mean that tasks for multi-db like `db:create:other_db` will not be
generated. If the database.yml is actually unreadable it will blow up
during normal rake task calls.
Fixes #36540
|
|/ |
|
|\
| |
| |
| |
| | |
vishaltelangre/raise-record-invalid-when-associations-fail-to-save-due-to-uniqueness-failure
Fix: ActiveRecord::RecordInvalid is not raised when an associated record fails to #save! due to uniqueness validation failure
|
| |
| |
| |
| |
| |
| |
| |
| | |
fails to #save! due to uniqueness validation failure
Add tests
Fix tests failing due to introduction of uniquness rule added to Book model
|
| | |
|
| | |
|
|\ \
| | |
| | | |
Address test_statement_cache_with_in_clause failure
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
sort order
This failure is occasional, does not always reproduce.
```ruby
$ cd activerecord
$ bundle exec rake test_postgresql
... snip ...
....F
Failure:
ActiveRecord::BindParameterTest#test_statement_cache_with_in_clause [/home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:97]:
Expected: [1, 3]
Actual: [3, 1]
rails test home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:93
```
|
|/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
I think we should change this, but not in 6-0-stable since that's
already in RC and I was trying to only make changes that won't require
any app changes.
This reverts a portion of https://github.com/rails/rails/pull/36439 that
made all schema migration version numbers get dumped as an integer.
While it doesn't _really_ matter it did change behavior. We should bring
this back in 6.1 with a deprecation.
|
|\ \
| | |
| | | |
Add test cases to ensure deterministic order for ordinal methods
|
| | |
| | |
| | |
| | |
| | |
| | | |
Before 1340498d2, `order` with no-op value (e.g. `nil`, `""`) had broken
the contract of ordinal methods, which returns a result deterministic
ordered.
|
|\ \ \
| | | |
| | | | |
Stop serializing and parsing columns_hash in Active Record schema caches
|
| |/ / |
|
|/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
When a record does not have a table name, as in the case for a record
with `self.abstract_class = true` and no `self.table_name` set the error
message raises a cryptic:
"ActiveRecord::StatementInvalid: Could not find table ''" this patch now
raises a new `TableNotSpecified Error`
Fixes: #36274
Co-Authored-By: Eugene Kenny <elkenny@gmail.com>
|
|\ \
| | |
| | | |
Deduplicate various Active Record schema cache structures
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Real world database schemas contain a lot of duplicated data.
Some column names like `id`, `created_at` etc can easily be repeated
hundreds of times. Same for SqlTypeMetada, most database will contain
only a limited number of possible combinations.
This result in a lot of wasted memory.
The idea here is to make these data sctructures immutable, use a registry
to substitute similar instances with pre-existing ones.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Currently `type.serialize` and `connection.{quote|type_cast}` for a time
object always does `time.getutc` call regardless of whether it is
already utc time object or not, that duplicated proccess
(`connection.type_cast(type.serialize(time))`) allocates extra/useless
time objects for each type casting.
This avoids that redundant `time.getutc` call if it is already utc time
object. In the case of a model has timestamps (`created_at` and
`updated_at`), it avoids 6,000 time objects allocation for 1,000 times
`model.save`.
```ruby
ObjectSpace::AllocationTracer.setup(%i{path line type})
pp ObjectSpace::AllocationTracer.trace {
1_000.times { User.create }
}.select { |k, _| k[0].end_with?("quoting.rb", "time_value.rb") }
```
Before (c104bfe424e6cebe9c8e85a38515327a6c88b1f8):
```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
203,
:T_ARRAY]=>[1004, 0, 778, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
220,
:T_STRING]=>[2, 0, 2, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
209,
:T_ARRAY]=>[8, 0, 8, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
57,
:T_ARRAY]=>[4, 0, 4, 1, 1, 0],
["~/rails/activemodel/lib/active_model/type/helpers/time_value.rb",
17,
:T_DATA]=>[4000, 0, 3096, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
120,
:T_DATA]=>[2000, 0, 1548, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
126,
:T_STRING]=>[4000, 0, 3096, 0, 1, 0]}
```
After (this change):
```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
203,
:T_ARRAY]=>[1004, 0, 823, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
220,
:T_STRING]=>[2, 0, 2, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
209,
:T_ARRAY]=>[8, 0, 8, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
57,
:T_ARRAY]=>[4, 0, 4, 1, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb",
126,
:T_STRING]=>[2000, 0, 1638, 0, 1, 0]}
```
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
GROUP BY with virtual count attribute is invalid for almost all
databases, but it is valid for PostgreSQL, and it had worked until Rails
5.2.2, so it is a regression for Rails 5.2.3 (caused by 311f001).
I can't find perfectly solution for fixing this for now, but I would not
like to break existing apps, so I decided to allow referencing virtual
count attribute in ORDER BY clause when GROUP BY aggrigation (it partly
revert the effect of 311f001) to fix the regression #36022.
Fixes #36022.
|
| | |
| | |
| | |
| | | |
`Arel::Attributes.for` is no longer used since https://github.com/rails/arel/pull/196.
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Each `visit o, collector` allocates one extra array due to
receiving args by splat array.
https://github.com/rails/rails/blob/2c3332cc4c0fa77dbe2e13e8a792f80fbd8f4ad3/activerecord/lib/arel/visitors/visitor.rb#L27-L29
Currently 1,000 times `User.where(id: 1).to_sql` allocates 13,000
arrays in `visitor.accept`. This avoids receiving args by splat array,
it makes `visitor.accept` no array allocation.
```ruby
ObjectSpace::AllocationTracer.setup(%i{path line type})
pp ObjectSpace::AllocationTracer.trace {
1_000.times { User.where(id: 1).to_sql }
}.select { |k, _| k[2] == :T_ARRAY && k[0].end_with?("visitor.rb", "to_sql.rb") }
```
Before (2c3332cc4c0fa77dbe2e13e8a792f80fbd8f4ad3):
```
{["~/rails/activerecord/lib/arel/visitors/to_sql.rb",
18,
:T_ARRAY]=>[1000, 0, 0, 0, 0, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb",
11,
:T_ARRAY]=>[1000, 0, 0, 0, 0, 0],
["~/rails/activerecord/lib/arel/visitors/visitor.rb",
12,
:T_ARRAY]=>[1000, 0, 0, 0, 0, 0],
["~/rails/activerecord/lib/arel/visitors/to_sql.rb",
788,
:T_ARRAY]=>[3000, 0, 0, 0, 0, 0],
["~/rails/activerecord/lib/arel/visitors/to_sql.rb",
794,
:T_ARRAY]=>[3000, 0, 0, 0, 0, 0],
["~/rails/activerecord/lib/arel/visitors/to_sql.rb",
156,
:T_ARRAY]=>[1000, 0, 0, 0, 0, 0],
["~/rails/activerecord/lib/arel/visitors/to_sql.rb",
443,
:T_ARRAY]=>[1000, 0, 0, 0, 0, 0],
["~/rails/activerecord/lib/arel/visitors/to_sql.rb",
603,
:T_ARRAY]=>[1000, 0, 0, 0, 0, 0],
["~/rails/activerecord/lib/arel/visitors/to_sql.rb",
611,
:T_ARRAY]=>[1000, 0, 0, 0, 0, 0]}
```
After (this change):
```
{}
```
|
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Tables in tests are not always empty so `klass.first` does not always
find last created record.
Fixes #36479.
|
| | |
| | |
| | |
| | | |
https://buildkite.com/rails/rails/builds/61744#f12cc6cf-7458-4131-917a-9735615f6259/999-1010
|
| | | |
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
If we put the `while_preventing_writes` on the connection then the
middleware that sends reads to the primary and ensures they can't write
will not work. The `while_preventing_writes` will only be applied to the
connection which it's called on - which in the case of the middleware is
Ar::Base.
This worked fine if you called it directly like
`OtherDbConn.connection.while_preventing_writes` but Rails didn't have a
way of knowing you wanted to call it on all the connections.
The change here moves the `while_preventing_writes` method from the
connection to the handler so that it can block writes to all queries for
that handler. This will apply to all the connections associated with
that handler.
|
|\ \ \
| | | |
| | | |
| | | |
| | | | |
eileencodes/move-schema-migration-to-migration-context
Move SchemaMigration to migration_context
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
This PR moves the `schema_migration` to `migration_context` so that we
can access the `schema_migration` per connection.
This does not change behavior of the SchemaMigration if you are using
one database. This also does not change behavior of any public APIs.
`Migrator` is private as is `MigrationContext` so we can change these as
needed.
We now need to pass a `schema_migration` to `Migrator` so that we can
run migrations on the right connection outside the context of a rake
task.
The bugs this fixes were discovered while debugging the issues around
the SchemaCache on initialization with multiple database. It was clear
that `get_all_versions` wouldn't work without these changes outside the
context of a rake task (because in the rake task we establish a
connection and change AR::Base.connection to the db we're running on).
Because the `SchemaCache` relies on the `SchemaMigration` information we
need to make sure we store it per-connection rather than on
ActiveRecord::Base.
[Eileen M. Uchitelle & Aaron Patterson]
|
|\ \ \ \
| |/ / /
|/| | |
| | | |
| | | | |
albertoalmagro/alberto/reverse-column-is-reversible
[ci skip] Update docs as `remove_column` can be reversed
|
| | | |
| | | |
| | | |
| | | |
| | | | |
As `remove_column` can be reversed when a type is provided this example
was not accurate anymore.
|
| | | | |
|
|/ / /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Currently 1,000 transactions creates 10,000 objects regardless whether
it is necessary or not.
This makes allocation on demand in transactions, now 1,000 transactions
creates required 5,000 objects only by default.
```ruby
ObjectSpace::AllocationTracer.setup(%i{path line type})
pp ObjectSpace::AllocationTracer.trace {
1_000.times { User.create }
}.select { |k, _| k[0].end_with?("transaction.rb") }
```
Before (95d038f):
```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
209,
:T_HASH]=>[1000, 0, 715, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
210,
:T_OBJECT]=>[1000, 0, 715, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
210,
:T_HASH]=>[1000, 0, 715, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
80,
:T_OBJECT]=>[1000, 0, 715, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
8,
:T_ARRAY]=>[1000, 0, 715, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
81,
:T_ARRAY]=>[1000, 0, 715, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
289,
:T_STRING]=>[1000, 0, 714, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
116,
:T_ARRAY]=>[1000, 0, 714, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
120,
:T_ARRAY]=>[1000, 0, 714, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
121,
:T_HASH]=>[1000, 0, 714, 0, 1, 0]}
```
After (this change):
```
{["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
213,
:T_HASH]=>[1000, 0, 739, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
214,
:T_OBJECT]=>[1000, 0, 739, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
214,
:T_HASH]=>[1000, 0, 739, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
81,
:T_OBJECT]=>[1000, 0, 739, 0, 1, 0],
["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb",
304,
:T_STRING]=>[1000, 0, 738, 0, 1, 0]}
```
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
* Make ActiveRecord `ConnectionPool.connections` thread-safe.
ConnectionPool documentation is clear on the need to synchronize
access to @connections but also states that public methods do not
require synchronization. Existing code exposed @connections
directly via attr_reader. The fix uses synchronize() to lock
@connections then returns a copy to the caller using Array.dup().
Includes comments on the connections method that thread-safe access
to the connections array does not imply thread-safety of accessing
methods on the actual connections.
Adds a test-case that modifies the pool using a supported method
in one thread while a second thread accesses pool.connections.
The test fails without this patch.
Fixes #36465.
* Update activerecord/test/cases/connection_pool_test.rb
[jeffdoering + Rafael Mendonça França]
|
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
"schema_migrations" table may be hard dropped before, so the reset
migration version should be done in ensure block.
https://buildkite.com/rails/rails/builds/61697#18d6f3ac-2257-4f4b-8efc-4010464c4d9a/999-1011
|
| | |
| | |
| | |
| | | |
https://buildkite.com/rails/rails/builds/61695#373bb1a7-677f-49ec-95e7-a92467fefd60/1076-1084
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
"schema_migrations" is hard dropped by some existing tests, so testing
migration in using transactional tests may cause implicit creation and
rollback "schema_migrations" table, it makes migration tests flaky.
https://buildkite.com/rails/rails/builds/61692#42383249-30be-4508-b1fb-a7bb27600c8e/999-1010
https://buildkite.com/rails/rails/builds/61694#6e462ad3-41d8-4e26-95ce-728495b0ac64/999-1010
|
|\ \ \
| | | |
| | | | |
Enable `Layout/EmptyLinesAroundAccessModifier` cop
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
We sometimes say "✂️ newline after `private`" in a code review (e.g.
https://github.com/rails/rails/pull/18546#discussion_r23188776,
https://github.com/rails/rails/pull/34832#discussion_r244847195).
Now `Layout/EmptyLinesAroundAccessModifier` cop have new enforced style
`EnforcedStyle: only_before` (https://github.com/rubocop-hq/rubocop/pull/7059).
That cop and enforced style will reduce the our code review cost.
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
`table_exists?` is already exist in `ModelSchema`.
https://github.com/rails/rails/blob/5cab344494c340ea82a35b46efa06b94f0b7730b/activerecord/lib/active_record/model_schema.rb#L339-L341
|
|/ / /
| | |
| | |
| | | |
Otherwise `Model.table_exists?` returns the staled cache result.
|
|\ \ \
| | | |
| | | | |
Add support for multiple databases to `rails db:abort_if_pending_migrations`
|
| | | | |
|
|\ \ \ \
| |/ / /
|/| | |
| | | |
| | | | |
kamipo/allow_column_name_with_simple_function_call
Allow column name with function (e.g. `length(title)`) as safe SQL string
|
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | |
| | | | |
Currently, almost all "Dangerous query method" warnings are false alarm.
As long as almost all the warnings are false alarm, developers think
"Let's ignore the warnings by using `Arel.sql()`, it actually is false
alarm in practice.", so I think we should effort to reduce false alarm
in order to make the warnings valuable.
This allows column name with function (e.g. `length(title)`) as safe SQL
string, which is very common false alarm pattern, even in the our
codebase.
Related 6c82b6c99, 6607ecb2a, #36420.
Fixes #32995.
|
|/ / / |
|
| | | |
|
| | |
| | |
| | |
| | |
| | | |
`split(/\s*,\s*/)` to order args and then `permit.match?` one by one is
much slower than `permit.match?` once.
|
| | |
| | |
| | |
| | | |
Method added in https://github.com/rails/rails/pull/36416
|
| | |
| | |
| | |
| | |
| | |
| | | |
6c82b6c99d86f37e61f935fb342cccd725d6c7d4
There is no need to be wrapped by `Arel.sql()`.
|
|\ \ \
| | | |
| | | | |
Fix preloading on AR::Relation where records are duplicated by a join
|
| | | | |
|