Commit 37d1429ab1d introduced the DummyERB to avoid loading the environment when
running `rake -T`.
The DummyCompiler simply replaced all output from `<%=` with a fixed string and
removed everything else. This worked okay when it was used for YAML values.
When using `<%=` within a YAML key, it caused an error in the YAML parser,
making it impossible to use ERB as you would expect. For example a
`database.yml` file containing the following should be possible:
development:
<% 5.times do |i| %>
shard_<%= i %>:
database: db/development_shard_<%= i %>.sqlite3
adapter: sqlite3
<% end %>
Instead of using a broken ERB compiler we can temporarily use a
`Rails.application.config` that does not raise an error when configurations are
accessed which have not been set as described in #35468.
This change removes the `DummyCompiler` and uses the standard `ERB::Compiler`.
It introduces the `DummyConfig` which delegates all known configurations to the
real `Rails::Application::Configuration` instance and returns a dummy string for
everything else. This restores the full ERB capabilities without compromising on
speed when generating the rake tasks for multiple databases.
Deprecates `config.active_record.suppress_multiple_database_warning`.
Prior to this change
t.virtual :column_name, type: :datetime
would erroneously produce the same result as
t.virtual :column_name, type: :datetime, precision: nil
This is because the code path for `virtual` skipped the default lookup:
- `t.datetime` is delegated to the `column` method
- `column` sees `type == :datetime` and sets a default `precision` is none was given
- `column` calls `new_column_definition` to add the result to `@columns_hash`
- `t.virtual` is delegated to the `column` method
- `column` sees `type == :virtual`, not `:datetime`, so skips the default lookup
- `column` calls `new_column_definition` to add the result to `@columns_hash`
- `new_column_definition` sees `type == :virtual`, so sets `type = options[:type]`
By moving the default lookup, we get consistent code paths:
- `t.datetime` is delegated to the `column` method
- `column` calls `new_column_definition` to add the result to `@columns_hash`
- `new_column_definition` sees `type == :datetime` and sets a default `precision` is none was given
- `t.virtual` is delegated to the `column` method
- `column` calls `new_column_definition` to add the result to `@columns_hash`
- `new_column_definition` sees `type == :virtual`, so sets `type = options[:type]`
- `new_column_definition` sees `type == :datetime` and sets a default `precision` is none was given
Problem:
Though, the `MessageVerifier` that powers `signed_id` supports both
`expires_in` and `expires_at`, `signed_id` only supports `expires_in`.
Because of this, generating signed_id that expires at a certain time is
somewhat tedious. Imagine issuing a coupon that is valid only for a
day.
Solution:
Add `expires_at` option to `signed_id` to generate signed ids that
expire at the given time.
Building on the work done in #44576 and #44591, we extend the logic that automatically
reconnects broken db connections to take into account a timeout limit. This ensures
that retries + reconnects are slow-query aware, and that we don't retry queries
if a given amount of time has already passed since the query was first tried.
This value will default to 5 seconds, but can be adjusted via the `connection_retry_timeout`
config.
This avoids problems when complex data structures are mutated _after_
being handed to ActiveRecord for processing. For example false hits in
the query cache.
Fixes#46044
According to the MySQL documentation, database connections default to
ssl-mode=PREFERRED. But PREFERRED doesn't verify the server's identity:
The default setting, --ssl-mode=PREFERRED, produces an encrypted
connection if the other default settings are unchanged. However, to
help prevent sophisticated man-in-the-middle attacks, it is
important for the client to verify the server’s identity. The
settings --ssl-mode=VERIFY_CA and --ssl-mode=VERIFY_IDENTITY are a
better choice than the default setting to help prevent this type of
attack. VERIFY_CA makes the client check that the server’s
certificate is valid. VERIFY_IDENTITY makes the client check that
the server’s certificate is valid, and also makes the client check
that the host name the client is using matches the identity in the
server’s certificate.
https://dev.mysql.com/doc/refman/8.0/en/using-encrypted-connections.html
However both the Rails::DBConsole command and the MySQLDatabaseTasks
ignore the ssl-mode option, making the connection fallback to PREFERRED.
Adding ssl-mode to the forwarded options makes sure the expected mode is
passed to the connection.
Followup to #45908 to match the same behavior as SchemaMigration
Previously, InternalMetadata inherited from ActiveRecord::Base. This is
problematic for multiple databases and resulted in building the code in
AbstractAdapter that was previously there. Rather than hacking around
the fact that InternalMetadata inherits from Base, this PR makes
InternalMetadata an independent object. Then each connection can get it's
own InternalMetadata object. This change required defining the methods
that InternalMetadata was depending on ActiveRecord::Base for (ex
create!). I reimplemented only the methods called by the framework as
this class is no-doc's so it doesn't need to implement anything beyond
that. Now each connection gets it's own InternalMetadata object which
stores the connection.
This change also required adding a NullInternalMetadata class for cases
when we don't have a connection yet but still need to copy migrations
from the MigrationContext. Ultimately I think this is a little weird -
we need to do so much work to pick up a set of files? Maybe something to
explore in the future.
Aside from removing the hack we added back in #36439 this change will
enable my work to stop clobbering and depending directly on
Base.connection in the rake tasks. While working on this I discovered
that we always have a ActiveRecord::InternalMetadata because the
connection is always on Base in the rake tasks. This will free us up
to do less hacky stuff in the migrations and tasks.
Both schema migration and internal metadata are blockers to removing
`Base.connection` and `Base.establish_connection` from rake tasks, work
that is required to drop the reliance on `Base.connection` which will
enable more robust (and correct) sharding behavior in Rails..
The issue fixed by the commit that introduced that entry only existed
in the main branch, so it isn't really a released change worthy of a
CHANGELOG entry.
Previously, SchemaMigration inherited from ActiveRecord::Base. This is
problematic for multiple databases and resulted in building the code in
AbstractAdapter that was previously there. Rather than hacking around
the fact that SchemaMigration inherits from Base, this PR makes
SchemaMigration an independent object. Then each connection can get it's
own SchemaMigration object. This change required defining the methods
that SchemaMigration was depending on ActiveRecord::Base for (ex
create!). I reimplemented only the methods called by the framework as
this class is no-doc's so it doesn't need to implement anything beyond
that. Now each connection gets it's own SchemaMigration object which
stores the connection. I also decided to update the method names (create
-> create_version, delete_by -> delete_version, delete_all ->
delete_all_versions) to be more explicit.
This change also required adding a NullSchemaMigraiton class for cases
when we don't have a connection yet but still need to copy migrations
from the MigrationContext. Ultimately I think this is a little weird -
we need to do so much work to pick up a set of files? Maybe something to
explore in the future.
Aside from removing the hack we added back in #36439 this change will
enable my work to stop clobbering and depending directly on
Base.connection in the rake tasks. While working on this I discovered
that we always have a `ActiveRecord::SchemaMigration` because the
connection is always on `Base` in the rake tasks. This will free us up
to do less hacky stuff in the migrations and tasks.
Following on #45924 I realized that `all_connection_pools` and
`connection_pool_list` don't make much sense as separate methods and
should follow the same deprecation as the other methods on the handler
here. So this PR deprecates `all_connection_pools` in favor of
`connection_pool_list` with an explicit argument of the role or `:all`.
Passing `nil` will throw a deprecation warning to get applications to
be explicit about behavior they expect.
Previously when I implemented multiple database roles in Rails there
were two handlers so it made sense for the methods
`active_connections?`, `clear_active_connections!`,
`clear_reloadable_connections!`, `clear_all_connections!`, and
`flush_idle_connections!` to only operate on the current (or passed)
role and not all pools regardless of role. When I removed this and moved
all the pools to the handler maintained by a pool manager, I left these
methods as-is to preserve the original behavior.
This made sense because I thought these methods were only called by
applications and not called by Rails. I realized yesterday that some of
these methods (`flush_idle_connections!`, `clear_active_connections!`,
and `clear_reloadable_connections!` are all called on boot by the
Active Record railtie.
Unfortunately this means that applications using multiple databases
aren't getting connections flushed or cleared on boot for any connection
but the writing ones.
The change here continues existing behavior if a role like reading is
passed in directly. Otherwise if the role is `nil` (which is the new
default` we fall back to all connections and issue a deprecation
warning. This will be the new default behavior in the future. In order
to easily allow turning off the deprecation warning I've added an `:all`
argument that will use all pools but no warning. The deprecation warning
will only fire if there is more than one role in the pool manager,
otherwise we assume prior behavior.
This bug would have only affected applications with more than one role
and only when these methods are called outside the context of a
`connected_to` block. These methods no longer consider the set
`current_role` and applications need to be explicit if they don't want
these methods to operate on all pools.
Add ability to use hash with columns and aliases inside #select method.
Post
.joins(:comments)
.select(
posts: { id: :post_id, title: :post_title },
comments: { id: :comment_id, body: :comment_body}
)
instead
Post
.joins(:comments)
.select(
"posts.id as post_id, posts.title as post_title,
comments.id as comment_id, comments.body as comment_body"
)
Co-authored-by: Josef Šimánek <193936+simi@users.noreply.github.com>
Co-authored-by: Jean byroot Boussier <19192189+casperisfine@users.noreply.github.com>
When source and target classes have a different set of attributes adapts
attributes such that the extra attributes from target are added.
Fixes#41195
Co-authored-by: SampsonCrowley <sampsonsprojects@gmail.com>
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
The `remove_check_constraint` method now accepts an `if_exists` option. If set
to true an error won't be raised if the check constraint doesn't exist.
This commit is a combination of PR #45726 and #45718 with some
additional changes to improve wording, testing, and implementation.
Usage:
```ruby
remove_check_constraint :products, name: "price_check", if_exists: true
```
Fixes#45634
Co-authored-by: Margaret Parsa <mparsa@actbluetech.com>
Co-authored-by: Aditya Bhutani <adi_bhutani16@yahoo.in>
Using `create_or_find_by` in codepaths where most of the time
the record already exist is wasteful on several accounts.
`create_or_find_by` should be the method to use when most of the
time the record doesn't already exist, not a race condition safe
version of `find_or_create_by`.
To make `find_or_create_by` race-condition free, we can search
the record again if the creation failed because of an unicity
constraint.
Co-Authored-By: Alex Kitchens <alexcameron98@gmail.com>
Fix: https://github.com/rails/rails/issues/45585
There's no benefit in serializing it as HWIA, it requires
to allow that type for YAML safe_load and takes more space.
We can cast it back to a regular hash before serialization.
https://github.com/rails/rails/pull/41395 added support for the `timestamptz` type on the Postgres adapter.
As we found [here](https://github.com/rails/rails/pull/41084#issuecomment-1056430921) this causes issues because in some scenarios the new type is not considered a time zone aware attribute, meaning values of this type in the DB are presented as a `Time`, not an `ActiveSupport::TimeWithZone`.
This PR fixes that by ensuring that `timestamptz` is always a time zone aware type, for Postgres users.
Previously Rails would always remove the connection if it found a
matching class in the pool manager. Therefore if
`ActiveRecord::Base.establish_connection` was called with the same
config, each time it was called it would be clobbered, even though the
config hasn't changed and the existing connection is prefectly fine. As
far as I can tell from conversations and reading the history this
functionality was added for ActiveRecord tests to be able to clobber the
connection and use a new config, then re-establish the old connection.
Essentially outside Rake tasks and AR tests, this functionality doesn't
have a ton of value.
On top of not adding a ton of value, this has resulted in a few bugs. In
Rails 6.0 I made it so that if you established a connection on
`ApplicationRecord` Rails would treat that connection the same as
`ActiveRecord::Base.` The reason for this is that the Railtie
establishes a connection on boot to the first database, but then if
you're using multiple databases you're calling `connects_to` in your
`ApplicationRecord` or primary abstract class which essentially doubles
your connections to the same database. To avoid opening 2 connections to
the same database, Rails treats them the same.
However, because we have this code that removes existing connections,
when an application boots, `ApplicationRecord` will clobber the
connection that the Railtie established even though the connection
configs are the same.
This removal of the connection caused bugs in migrations that load up a
model connected to `ApplicationRecord` (ex `Post.first`) and then calls
`execute("SELECT 1")` (obviously a simplified example). When `execute`
runs the connection is different from the one opened to run the
migration and essentially it is lost when the `remove_connection` code
is called.
To fix this I've updated the code to only remove the connection if the
database config is different. Ultimately I'd like to remove this code
altogether but to do that we first need to stop using
`Base.establish_connection` in the rake tasks and tests. This will fix
the major bugs until I come up with a solution for the areas that
currently need to call `establish_connection` on Base.
The added benefit of this change is that if your app is calling
`establish_connection` multiple times with the same config, it is now
3x faster than the previous implementation because we can return the
found pool instead of setting it up again. To benchmark this I
duplicated the `establish_connection` method to use the new behavior
with a new name.
Benchmark script:
```ruby
require "active_record"
require "logger"
require "benchmark/ips"
config_hash = { "development" => { "primary" => { "adapter" => "mysql2", "username" => "rails", "database" => "activerecord_unittest"}}}
ActiveRecord::Base.configurations = config_hash
db_config = ActiveRecord::Base.configurations.configs_for(env_name: "development", name: "primary")
p "Same model same config"
ActiveRecord::Base.connected_to(role: :writing, prevent_writes: true) do
Benchmark.ips do |x|
x.report "establish_connection with remove" do
ActiveRecord::Base.establish_connection(db_config)
end
x.report "establish_connection without remove" do
ActiveRecord::Base.establish_connection_no_remove(db_config)
end
x.compare!
end
end
```
Benchmark results:
```
Warming up --------------------------------------
establish_connection with remove
4.677k i/100ms
establish_connection without remove
19.501k i/100ms
Calculating -------------------------------------
establish_connection with remove
41.252k (±11.3%) i/s - 205.788k in 5.075525s
establish_connection without remove
179.205k (± 6.9%) i/s - 897.046k in 5.029742s
Comparison:
establish_connection without remove: 179205.1 i/s
establish_connection with remove: 41252.3 i/s - 4.34x (± 0.00) slower
```
Other changes:
1) sqlite3 now disconnects and reconnects the connection when `purge` is
called. This is necessary now that a new connection isn't created
everyt time `establish_connection` is called. Without this change to
purge the new database is left in an inaccessible state causing a
readonly error from the sqlite3 client. This wasn't happening in mysql
or postgres because they were already reconnecting the db connection.
2) I added `remove_connection` to tests that use `ApplicationRecord`.
This is required because `ApplicationRecord` or any class that is a
`primary_abstract_class` will be treated the same as
`ActiveRecord::Base`. This is fine in applications because they are
shared connections, but in the AR test environment, we don't want those
connnections to stick around (we want AR::Base back).
3) In the async tests I removed 2 calls to `establish_connection`. These
were causing sqlite3 tests to leak the state of async_executor because
it's stored on the connection. I'm not sure why these were calling
`establish_connection` but it's not necessary and was leaking state when
now that we are no longer removing the connection.
Fixes: #41855Fixes: #41876Fixes: #42873Fixes: #43004
The default strategy will continue to forward messages to the connection adapter,
but applications can configure a custom strategy class to use instead.
This removes the singularize from `where` which runs on all `expand_from_hash` keys which might be reflections or column names. This saves a lot of time by avoiding singularizing column names.
Previously in https://github.com/rails/rails/pull/45163 the singularize was removed entirely. after some reflection, I think it is better to at least give a warning for one release since `where` is a very popular API and the problems you can run into with incorrect relation could be hard to debug.
Configurable with `ActiveRecord::Base.allow_deprecated_singular_assocaitions_name = false` / `config.active_record.allow_deprecated_singular_assocaitions_name = false`
Currently, when #reset is called on a relation object it does not reset
the cache_versions ivar. This can lead to a confusing and buggy
situation where despite having the correct data the relation is still
reporting a stale cache_version. Resetting this ivar along with the
other relation state corrects this situation.
Update test assertion
Assert the specific cache_version matches the latest .all relation
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
