This commit fixes the issue where the `primary_key:` option was ignored
if the associated model had a `query_constraints` configured.
Now `primary_key:` option always takes priority and only if there is no
`primary_key:` option, the `query_constraints` are used to determine
the `association_primary_key` value.
Following up on https://github.com/rails/rails/pull/49192, this commit
adds the transaction `outcome` to the payload, helpful for collecting
stats on how many transactions commit, rollback, restart, or (perhaps
most interestingly) are incomplete because of an error.
The one quirk here is that we have to modify the payload on finish. It's
not the only place this sort of thing happens (instrument mutates the
payload with exceptions, for example), but it does mean we need to dup
the payload we initialize with to avoid mutating it for other tracking.
Co-authored-by: Ian Candy <ipc103@github.com>
We already have access to the `active_record` on the reflection here so
there's no point in passing it to `derive_fk_query_constraints`.
In addition the id, fk check wasn't actually doing anything here, it was
holdover from debugging I was doing when implementing this
functionality.
Tracking Active Record-managed transactions seems to be a common need,
but there's currently not a great way to do it. Here's a few examples
I've seen:
* GitHub has custom transaction tracking that monkey patches the Active
Record `TransactionManager` and `RealTransaction`. We use the tracking
to prevent opening a transaction to one database cluster inside a
transaction to a different database cluster, and to report slow
transactions (we get slow transaction data directly from MySQL as well,
but it's still helpful to report from the application with backtraces to
help track them down).
* https://github.com/palkan/isolator tracks transactions to prevent
non-atomic interactions like external network calls inside a
transaction. The gem works by subscribing to `sql.active_record`, then
piecing together the transactions by looking for `BEGIN`, `COMMIT`,
`SAVEPOINT`, etc., but this is unreliable:
- https://github.com/palkan/isolator/issues/65
- https://github.com/palkan/isolator/issues/64
* It looks like GitLab patches `TransactionManager` and `RealTransaction`
to track nested savepoints. See https://github.com/palkan/isolator/issues/46
This commit adds a new `transaction.active_record` event that should
provide a more reliable solution for these various use cases. It
includes the connection in the payload (useful, for example, in
differentiating transactions to different databases), but if this change
gets merged we're also planning to add details about what type of
transaction it is (savepoint or real) and what the outcome is (commit,
rollback, restarted, errored).
This instrumentation needs to start and finish at fairly specific times:
- start on materialize
- finish after committing or rolling back, but before the after_commit
or after_rollback callbacks
- finish and start again when the transaction restarts (at least for
real transactions—we've done it for savepoints as well but I'm not
certain we should)
- ensure it finishes if commit and rollback fail (e.g. if the
connection goes away)
To make all that work, this commit uses the lower-level `#build-handle`
API instead of `#instrument`.
Co-authored-by: Ian Candy <ipc103@github.com>
RDoc was confused by the placement of `:nodoc:`, causing it to skip the
documentation for `ActiveRecord::TokenFor#generate_token_for` (though
not the documentation for `ActiveRecord::TokenFor::ClassMethods`).
Also, RDoc supports `<b>` tags but not `<strong>` tags. (Though,
ironically, it converts `<b>` tags to `<strong>` tags when rendering.)
In #49105, `where` examples were added to the `normalizes` documentation
to demonstrate that normalizations are also applied for `where`.
However, as with the `exists?` examples, we should also demonstrate that
normalizations are only applied to keyword arguments, not positional
arguments. We can also address the original source of the confusion by
changing the wording of "finder methods" to "query methods".
This commit also removes the tests added in #49105. `normalizes` works
at the level of attribute types, so there is no need to test every query
method. Testing `find_by` is sufficient. (And, in point of fact,
`find_by` is implemented in terms of `where`.)
The `add_check_constraint` method now accepts an `if_not_exists` option. If set to true an error won't be raised
if the check constraint already exists. In addition, `if_exists` and `if_not_exists` options are transposed
if set when reversing `remove_check_constraint` and `add_check_constraint`. This enables simple creation
of idempotent, non-transactional migrations.
`undefine_attribute_methods` now removes alias attribute methods along
with attribute methods. This commit changes `define_attribute_methods` to
redefine methods back if any alias attributes were declared which provides
applications and libraries an option to bring the alias methods back
after using `undefine_attribute_methods`.
Previously, a composite primary key model would need to specify
either a `primary_key` or `query_constraints` option on its associations.
Without it, a `CompositePrimaryKeyMismatchError` would be raised.
Most of the time, the composite primary key includes the `:id` column,
and associations already expect that `:id` is being used as the
primary key for the association (and have a corresponding foreign_key
column set on the associated model).
Rather than requiring users to define `primary_key: :id` throughout
an application with composite primary key associations, we can infer
that the `:id` column should be used. Users can still override this
behaviour by specifying a `primary_key:` or `query_constraints:` on
the association.
Note that, if the composite primary key for a model does _not_ include
`:id`, Rails won't infer the primary key for any related associations,
and users must still specify `query_constraints` or `primary_key`.
Prior to this change, you'd need to do the following to set up
associations for composite primary key models:
```ruby
class Order
self.primary_key = [:shop_id, :id]
has_many :order_agreements, primary_key :id
end
class OrderAgreement
belongs_to :order, primary_key: :id
end
```
After this change, the `primary_key` option no longer needs to be
specified:
```ruby
class Order
self.primary_key = [:shop_id, :id]
has_many :order_agreements
end
class OrderAgreement
belongs_to :order
end
```
Follow-up to #47420.
Whereas the original behavior (`on: :create`) is invoked only once
before a record is persisted, the new behavior (`on: :initialize`) is
invoked not only new record but also persisted records.
It should be invoked only once for new record consistently.
Follow-up to [#47420][]
With the changes made in [#47420][], `has_secure_token` declarations can
be configured to execute in an `after_initialize` callback. This commit
proposed a new Rails 7.1 default: generate all `has_secure_token` values
when their corresponding models are initialized.
To preserve pre-7.1 behavior, applications can set
`config.active_record.generate_secure_token_on = :create`.
By default, generate the value when the model is initialized:
```ruby
class User < ApplicationRecord
has_secure_token
end
record = User.new
record.token # => "fwZcXX6SkJBJRogzMdciS7wf"
```
With `config.active_record.generate_secure_token_on = :create`, generate
the value when the model is created:
```ruby
# config/application.rb
config.active_record.generate_secure_token_on = :create
# app/models/user.rb
class User < ApplicationRecord
has_secure_token on: :create
end
record = User.new
record.token # => nil
record.save!
record.token # => "fwZcXX6SkJBJRogzMdciS7wf"
```
[#47420]: https://github.com/rails/rails/pull/47420
Co-authored-by: Hartley McGuire <skipkayhil@gmail.com>
There were a few 6.1 migration compatibility fixes in [previous][1]
[commits][2]. Most importantly, those commits reorganized some of the
compatibility tests to ensure that the tests would run against every
Migration version. To continue the effort of improving test coverage for
Migration compatibility, this commit converts tests for create_table and
change_column setting the correct precision on datetime columns.
While the create_table tests all pass, the change_column test did not
pass for 7.0 versioned Migrations on sqlite. This was due to the sqlite
adapter not using new_column_definition to set the options on the new
column (new_column_definition is where precision: 6 gets set if no
precision is specified). This happens because columns can't be modified
in place in sqlite and instead the whole table must be recreated and the
data copied. Before this commit, change_column would use the options
of the existing column as a base and merge in the exact options (and
type) passed to change_column.
This commit changes the change_column method to replace the existing
column without using the existing options. This ensures that precision:
6 is set consistently across adapters when change_column is used to
create a datetime column.
[1]: c2f838e80c
[2]: 9b07b2d6ca
I noticed this test was throwing a warning for `alias_attribute
:id_value, :id` defined by Rails. Since this test messes with primary
keys I decided to fix it by making custom classes for each of the
tests that need to change the PK.
While debugging these tests I noticed the deprecation wasn't formatted
correctly so I fixed that while in there.
Active Record already has the ability to guess the foreign key. With
query constraints we can use the information from the owner class and
that foreign key to infer what the query constraints should be for an
association. This change improves the ergonomics of the query
constraints feature by making it so that you only have to define the
query constraints on the top level model and skip defining it on the
association. There are a few caveats to this as of this PR:
- If the query constraints on the parent are greater than 2, Rails can't
derive the association constraints
- If the query constraints on the parent don't include the primary key,
Rails can't derive the association constraints.
- If the association has an explicit `foreign_key` or
`query_constraints` option set, Active Record won't infer the key.
- I have not implemented support for CPK yet as it's more complex and
maybe not possible since we don't know which key to split on and use
for the foreign association.
Prior to this change you would need to do the following to use query
constraints:
```ruby
class Post
query_constraints :blog_id, :id
has_many :comments, query_constraints: [:blog_id, :blog_post_id]
end
class Comment
query_constraints :blog_id, :id
belongs_to :blog, query_constraints: [:blog_id, :blog_post_id]
end
```
After this change, the associations don't require you to set the query
constraints, Active Record will generate `[:blog_id, :blog_post_id]`
foreign key itself.
```ruby
class Post
query_constraints :blog_id, :id
has_many :comments
end
class Comment
query_constraints :blog_id, :id
belongs_to :blog
end
```
`Kernel#caller` has linear performance based on how deep the
stack is. While this is a development only feature, it can end
up being quite slow.
Ruby 3.2 introduced `Thread.each_caller_location`, which lazily
yield `Backtrace::Location` objects.
Ref: https://bugs.ruby-lang.org/issues/16663
This is perfect for this use case as we are searching for the
closest frame that matches a pattern, saving us from collecting
the entire backtrace.
We ran into a few cases at GitHub where we were using alias_attribute
incorrectly and the new behavior either didn't warn or raised an unclear
deprecation warning. This attempts to add clarity to the deprecation reason
when you try to alias something that isn't actually an attribute.
Previously, trying to alias a generated attribute method, such as `attribute_in_database`, would
still hit `define_proxy_call`, because we were only checking the owner of the target method.
In the future, we should probably raise if you try to use alias_attribute for a non-attribute.
Note that we don't raise the warning for abstract classes, because the attribute may be implemented
by a child class. We could potentially figure out a way to raise in these cases as well, but this
hopefully is good enough for now.
Finally, I also updated the way we're setting `local_alias_attributes` when `alias_attribute` is
first called. This was causing problems since we now use `alias_attribute` early in the
`load_schema!` call for some models: https://buildkite.com/rails/rails/builds/98910
This commit deprecates `read_attribute(:id)` returning the primary key
if the model's primary key is not the id column. Starting in Rails 7.2,
`read_attribute(:id)` will always return the value of the id column.
This commit also changes `read_attribute(:id)` for composite primary
key models to return the value of the id column, not the composite
primary key.
When an abstract class inherited from an Active Record model
defines an alias attribute Rails should expect the original methods
of the aliased attribute to be defined in the parent class and avoid
raising deprecation warning.
This is similar to a [previous commit][1] which ensures that versioned
migrations always call `super` in `compatible_table_definition`. In this
case, these methods are being pulled up to `Current` so that all
subclasses will use a `TableDefinition` class and future developers do
not have to remember to add all of these methods to new versioned
classes when a new one is created.
[1]: 16f8bd7944
While working on #48969, I found that some of the Compatibility test
cases were not working correctly. The tests removed in this commit were
never running the `change_table` migration and so were not actually
testing that `change_table` works correctly. The issue is that the two
migrations created in these tests both have `nil` versions, and so the
Migrator only runs the first one.
This commit refactors the tests so that its easier to test the behavior
of each Migration class version (and I think the rest of the tests
should be updated to use this strategy as well). Additionally, since the
tests are fixed it exposed that `t.change` in a `change_table` is not
behaving as expected so that is fixed as well.
This commit adds support for composite identifiers in `to_key`.
Rails 7.1 adds support for composite primary key which means that
composite primary key models' `#id` method returns an `Array` and
`to_key` needs to avoid double-wrapping the value.
This allows access to the raw id column value on records for which an
id column exists but is not the primary key. This is common amongst
models with composite primary keys.
rails/rails@a88f47d fixed an issue where subclasses were regenerating aliased attribute methods defined on parent classes. Part of the solution was to call #generate_alias_attributes recursively on a class's superclass to generate all the alias attributes in the inheritance hierarchy. However, the implementation relies on #base_class to determine if we should call #generate_alias_attributes on the superclass. Since all models that inherit from abstract classes are base classes, this means that #generate_alias_attributes will never be called on abstract classes, meaning no method will be generated for any alias attributes defined on them.
To fix this issue, we should always call #generate_alias_attributes on the superclass unless the superclass is ActiveRecord::Base.
I'm working on an enhancement to query constraints that will require me
to know when we have query constraints but not a composite key.
Currently if you have a composite key it will be included in the query
constraints list. There's not a way to differentiate between the two
which means that we're forced into setting the query constraints on the
associations for the primary key.
This change adds a `has_query_constraints?` method so we can check the
class for query constraints. The options still work as well but we can
be sure we're always picking up the query constraints when they're
present.
We originally implemented this project to use OR queries instead of IN
because it seemed like there was no way to do that without a row
constructor. While working on implementing this feature in our vitess
gem I noticed that we can actually get the queries we wanted, and the
new code is more performant than generating an OR.
SQL Before:
```sql
SELECT "sharded_comments".*
FROM "sharded_comments"
WHERE ("sharded_comments"."blog_id" = 969142904
AND ("sharded_comments"."blog_post_id" = 357271355
OR "sharded_comments"."blog_post_id" = 756811794)
OR "sharded_comments"."blog_id" = 308674288
AND "sharded_comments"."blog_post_id" = 1055755181)
```
SQL After:
```sql
SELECT "sharded_comments".*
FROM "sharded_comments"
WHERE "sharded_comments"."blog_id"
IN (969142904, 308674288)
AND "sharded_comments"."blog_post_id"
IN (357271355, 756811794, 1055755181)
```
Using one of the tests that utilizes this code path, I benchmarked the
queries. The new implementation is faster:
Before:
```
Warming up --------------------------------------
queries 147.000 i/100ms
Calculating -------------------------------------
queries 1.486k (± 3.2%) i/s - 7.497k in 5.050742s
```
After:
```
Warming up --------------------------------------
queries 179.000 i/100ms
Calculating -------------------------------------
queries 1.747k (± 4.5%) i/s - 8.771k in 5.031424s
```
We can probably improve this more but I think this query is more in line
with what we want and expect while also being more performant (without
having to build a new row constructor in arel).
Because we're using a class_attribute to track all of the attribute_aliases,
each subclass was regenerating the parent class methods, causing some unexpected
deprecation messages.
Instead, we can use a class instance variable to track the attributes aliased locally
in that particular subclass. We then walk up the chain and re-define the attribute methods
if they haven't been defined yet.
Pluses cannot be used to create code blocks when the content includes a
space.
Found using a regular expression:
```bash
$ rg '#\s[^+]*\+[^+]*\s[^+]*\S\+'
```
If you have already loaded the relation when using in_batches, you
would reload each batch from the database. Since we already have the
records available, we can avoid these queries and return the records
as-if the query was run.
Despite the inconvenience of double-backslashing, the backslash was chosen
because it's a very common char to use in escaping across multiple programming
languages. A developer, without looking at documentation, may intuitively try
to use it to achieve the desired results in this scenario.
Fixes#37779
AbstractAdapter#schema_version uses migration_context.current_version by
default, but some adapters might redefine this method to use a custom
schema version number. This fix ensures that generated schema cache
files use this method and that it is also used when validating a file's
version.
This PR fixes a bug introduced in https://github.com/rails/rails/pull/48761
which leads to a `NoMethodError` when a non Active Record object passed
to `include?` or `member?` since only Active Record objects
respond to `composite_primary_key?`.
Followup: https://github.com/rails/rails/pull/48743
After careful consideration, unless users have a schema cache dump loaded
and `check_schema_cache_dump_version = false`, we have no choice but
to arbitrate between resiliency and performance.
If we define attribute methods during boot, we allow them to be shared
between forked workers, and prevent the first requests to be slower.
However, by doing so we'd trigger a connection to the datase, which
if it's unresponsive could lead to workers not being able to restart
triggering a cascading failure.
Previously Rails used to be in some sort of middle-ground where
it would define attribute methods during boot if for some reason
it was already connected to the database at this point.
But this is now deemed undesirable, as an application initializer
inadvertantly establishing the database connection woudl lead to
a readically different behavior.
`include` and `member?` delegate to `exists?` with the record's primary
key to determine if an unloaded relation contains a given record. If
the primary key is composite, `exists?` belives we are passing
where-style conditions and fails.
This commit fixes the issue by turning the record's composite primary key
into a hash of column / value pairs that `exists?` can accept.
The callback when the value is generated. When called with `on:
:initialize`, the value is generated in an `after_initialize` callback,
otherwise the value will be used in a `before_` callback. It will
default to `:create`.
This commit changes bodies of methods generated by `alias_attribute`
along with generating these methods lazily.
Previously the body of the `alias_attribute :new_title, :title` was
`def new_title; title; end`. This commit changes it to
`def new_title; attribute("title"); end`.
This allows for `alias_attribute` to be used to alias attributes named
with a reserved names like `id`:
```ruby
class Topic < ActiveRecord::Base
self.primary_key = :title
alias_attribute :id_value, :id
end
Topic.new.id_value # => 1
```
#48530 introduced a problem for the system that extends AR queries involving deterministically encrypted attributes. The problem is that this option added the same "previous scheme" for all the attributes, when the only intended ones were the "non deterministic" ones. A side effect of this is that, when encrypting query param values, it was using the wrong encryption scheme.
Fixes https://github.com/rails/rails/issues/48204#issuecomment-1623301440
Followup: https://github.com/rails/rails/pull/48716
`model.connection_pool.schema_reflection` is never falsy, so that `if`
was pointless.
Instead we more properly check if the schema cache contains that table.
I also added some more comments to explain why the initializer tries
so hard not to touch the database.
Partially Fixes#48164. Followup to #48200.
`within_new_transaction` currently discards connections if an error
is raised which may have left the connection in a transaction. This change
updates that logic to discard connections in an `ensure` block to handle
cases where an error is not raised, for instance if the thread is killed.
A big problem with the current `SchemaCache` implementation is that it holds
a reference to a connection, and uses it to lazily query the schema when the
information is missing.
This cause issues in multi-threaded contexts because all the connections
of a pool share the same schema, so they are constantly setting themselves
as the connection the schema cache should use, and you can end up in
situation where Thread#1 query the schema cache, which end up using the
connection currently attributed to Thread#2.
For a very long time this worked more or less by accident, because all
connection adapters would have an internal Monitor.
However in https://github.com/rails/rails/pull/46519 we got rid of this
synchronization, revealing the pre-existing issue. Previously it would
work™, but still wasn't ideal because of unexpected the connection
sharing between threads.
The idea here is to refactor the schema cache so that it doesn't hold a
reference onto any connection.
And instead of a `SchemaCache`, connections now have access to a
`SchemaReflection`. Now the connection that needs a schema information
is always provided as an argument, so that in case of a miss, it can be
used to populate the cache.
That should fix the database thread safety issues that were witnessed.
However note that at this stage, the underlying `SchemaCache` isn't
synchronized, so in case of a race condition, the same schema query
can be performed more than once. It could make sense to add synchronization
in a followup.
This refactoring also open the door to query the cache without a connection,
making it easier to eagerly define model attribute methods on boot without
establishing a connection to the database.
Co-Authored-By: Jean Boussier <jean.boussier@gmail.com>
Co-Authored-By: zzak <zzakscott@gmail.com>
Fix when multiple `belongs_to` maps to the same counter_cache column.
In such situation `inverse_which_updates_counter_cache` may find the
wrong relation which leads into an invalid increment of the
counter_cache.
This is done by releying on the `inverse_of` property of the relation
as well as comparing the models the association points two.
Note however that this second check doesn't work for polymorphic
associations.
Fixes#41250
Co-Authored-By: Jean Boussier <jean.boussier@gmail.com>
A developer may not always want to specify query constraints. If, for
example they would like to use a non-composite primary key in an
asscoaition, they need to use primary_key/foreign_key options instead.
Co-authored-by: Nikita Vasilevsky <nikita.vasilevsky@shopify.com>
Support assignment of belongs_to associations with query constraints.
This improves errors messages for composite key mismatches, and some
edge cases with query constraint lists.
Co-authored-by: Nikita Vasilevsky <nikita.vasilevsky@shopify.com>
Catches more bugs in CPK assocaitions by validating the shape of keys on
both ends of the association.
Co-authored-by: Nikita Vasilevsky <nikita.vasilevsky@shopify.com>
The check for `Base.connected?` was added in
4ecdf24bde.
At the time this change was made, we were calling
`Base.connection.reset_runtime` so it made sense to check if the
database was connected. Since
dd61a817de
we have stored that information in a thread instead, negating the need
to check `Base.connected?`.
The problem with checking `Base.connected?` is that in a multi-db
application this will return `false` for any model running a query not
inheriting from `Base`, leaving the `db_runtime` payload `nil`. Since
the information we need is stored in a thread rather than on the
connection directly we can remove the `Base.connected?` check.
Fixes#48687
Fix: https://github.com/rails/rails/issues/45017
Ref: https://github.com/rails/rails/pull/29333
Ref: https://github.com/ruby/timeout/pull/30
Historically only raised errors would trigger a rollback, but in Ruby `2.3`, the `timeout` library
started using `throw` to interupt execution which had the adverse effect of committing open transactions.
To solve this, in Active Record 6.1 the behavior was changed to instead rollback the transaction as it was safer
than to potentially commit an incomplete transaction.
Using `return`, `break` or `throw` inside a `transaction` block was essentially deprecated from Rails 6.1 onwards.
However with the release of `timeout 0.4.0`, `Timeout.timeout` now raises an error again, and Active Record is able
to return to its original, less surprising, behavior.
Previously, when merging persisted and in-memory records, we were using
`record[name]` to get the value of the attribute. For records with a
composite primary key, the full CPK would be returned, which was
incompatible with `#_write_attribute`: we'd attempt to assign an Array
to the `id` column, which would result in the column being set to nil.
When dealing with a record with a composite primary key, we need to
write each part of the primary key individually via `#_write_attribute`.
The `name` argument is not useful as `remove_connection` should be called
on the class that established the connection. Allowing `name` to be
passed here doesn't match how any of the other methods behave on the
connection classes. While this behavior has been around for a long time,
I'm not sure anyone is using this as it's not documented when to use
name, nor are there any tests.
If anyone calls a cypher in the console it will show the secret of the
encryptor.
By overriding the `inspect` method to only show the class name we can
avoid accidentally outputting sensitive information.
Before:
```ruby
ActiveRecord::Encryption::Cipher::Aes256Gcm.new(secret).inspect
"#<ActiveRecord::Encryption::Cipher::Aes256Gcm:0x0000000104888038 ... @secret=\"\\xAF\\bFh]LV}q\\nl\\xB2U\\xB3 ... >"
```
After:
```ruby
ActiveRecord::Encryption::Cipher::Aes256Gcm(secret).inspect
"#<ActiveRecord::Encryption::Cipher::Aes256Gcm:0x0000000104888038>"
```
Allows building of records from an association with a has_one through a
singular association with inverse. For belongs_to through associations,
linking the foreign key to the primary key model isn't needed.
For has_one, we cannot build records due to the association not being mutable.
The existing process was attempting to detect duplicates by storing added records in `Set.new`.
When a record is added to `Set.new`, the identity is calculated using `ActiveRecord::Core#hash`, but this value changes before and after saving, so duplicates were not detected before and after saving even for the same object.
This PR fixed the problem by using the `#object_id` of the record to detect duplicates.
Note that when storing a new object obtained by `ActiveRecord::Base.find` etc., duplicates are not eliminated because the `#object_id` is different. This is the same behavior as the current `ActiveRecord::Associations::CollectionProxy#<<`.
Adds support for building records in has_many through has_one
composite primary key associations.
Also updates query constraints on associations affected by rails/rails#48564.
Fixes 2 bugs in parallel testing.
The first bug was related to changes made in #45450 which meant that we were
no longer replacing the connection in parallel testing because the
config object is equal (we simply merge a new db name but the object id
of the config stays the same). This bug only manifested in mysql and
sqlite3 interestingly. It would fail on the internal metadata tables
because they were missing in the schema version check.
To fix this I introduced a `clobber: true` kwarg onto the connection
handler that allows us to bypass the functionality that won't make a new
connection if the config is the same. This is an easy way to fall back
to the old behavior from before this change. I only added `clobber`
to the `reconstruct_from_schema` call because we need to actually
replace the connection for these. It's not safe to add everywhere since
we don't always want to replace the connection.
After implementing this fix I was still seeing failures in the mysql
demo app I made due to the fact that `purge` was not re-establishing the
connection to a config that had a database defined. Neither sqlite3 or
postgresql were missing this.
I added a test for mysql2 so we don't have regressions in the future. I
think this was missed because sqlite3 only demonstrates the bug if it
was never successful on that worker and postgresql was fine.
Fixes#48547
The change in PR #43302 introduced a bug where unsaved records are not displayed in pretty_print.
The following code will not show unsaved records until this PR is merged.
```ruby
post = Post.create!
post.comments.build
pp(post.comments) #=> expected "[#<Post:0x000000014c0b48a0 ...>]", got "[]"
```
Fixed to call `#load_target` before display as well as `#inspect`.
Fixes#48398
Prepared Statements and Query Logs are incompatible features due to query logs making every query unique.
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Currently it is a bit unclear which dirty methods can be called on
Active Record models. You have to know that methods from ActiveModel::Dirty
are included.
It also unclear if methods can be invoked in the form of
`saved_change_to_name?` unless you read the documentation of the
`saved_change_to_attribute?` method.
By adding an introduction to the module we can show which methods are
defined specifically for Active Record, and how to call them, very
similar to the ActiveModel::Dirty introduction.
Linking to ActiveModel::Dirty makes it's also easier to find methods
defined there.
This reverts commit 6264c1de76, reversing
changes made to 6c80bcdd20.
Reason: Still discussion about the feature. We want to make it opt-in
but we need to better understand why people would want to opt-in to
this behavior.
When eager_load is enabled and something (like the frozen_record gem)
tries to load yaml files that use `ActiveRecord::FixtureSet.identify(...)`
we'll get an uninitialized constant error.
Move requires for constants in the ActiveRecord::FixtureSet namespace
to below the creation of the class to avoid circular requires.
See ActiveSupport::Deprecation for a similar instance of this:
7dd67dc285/activesupport/lib/active_support/deprecation.rb (L35-L54)
Support primary key dirty tracking when ID is a composite
primary key with an id column.
Introduces `ActiveRecord::AttributeMethods::Query#_query_attribute`
to bypass id reader.
* Make sure active record encryption configuration happens after initializers have run
Co-authored-by: Cadu Ribeiro <mail@cadu.dev>
* Add a new option to support previous data encrypted non-deterministically with a hash digest of SHA1
There is currently a problem with Active Record encryption for users updating from 7.0 to 7.1 Before
#44873, data encrypted with non-deterministic encryption was always using SHA-1. The reason is that
`ActiveSupport::KeyGenerator.hash_digest_class` is set in an after_initialize block in the railtie config,
but encryption config was running before that, so it was effectively using the previous default SHA1. That
means that existing users are using SHA256 for non deterministic encryption, and SHA1 for deterministic
encryption.
This adds a new option `use_sha1_digest_for_non_deterministic_data` that
users can enable to support for SHA1 and SHA256 when decrypting existing data.
* Set a default value of true for `support_sha1_for_non_deterministic_encryption` and proper initializer values.
We want to enable the flag existing versions (< 7.1), and we want it to be false moving by
default moving forward.
* Make sure the system to auto-filter params supports different initialization orders
This reworks the system to auto-filter params so that it works when encrypted
attributes are declared before the encryption configuration logic runs.
Co-authored-by: Cadu Ribeiro <mail@cadu.dev>
---------
Co-authored-by: Cadu Ribeiro <mail@cadu.dev>
Composite primary key records need to conditionally unset multiple
column attributes from the associated record in order to properly
support dependent: nullify.
This reverts commit 8b36095881, reversing
changes made to e05245db87.
Railties tests have been failing since this change. The issue is that
calling `primary_key` as the model is loaded requires either a
connection to the database or a populated schema cache. This becomes an
issue when an app loads models that do not have underlying tables, as
shown in the failing Railties tests.
When eager loading an app using `rails/all`,
`ActionMailbox::InboundEmail` will be loaded whether or not `rails g
action_mailbox:install` has been run. This means the `primary_key` for
`InboundEmail` will not be in the schema cache and a database connection
will be required to boot the app.
* Improve the spacing of various paragraphs
* Link to AS::TestCase.fixture_paths= API
* Add a note about needing to require "rails/test_help" to get fixture_paths
* Expanded briefly on YAML default map unordering property
* Improve heading weight consistency
* Unlink several RDoc autolinked things
This removes the deprecated .fixture_path and .fixture_path= methods
from public documentation, and creates a class method for the getter and
setters.
Interestingly because these methods were always `class_attribute` based,
which RDoc doesn't parse, they weren't technically "public"
documentation.
Fixes https://github.com/rails/rails/issues/48524
The test case in the issue breaks because `value.respond_to?(:id)` returns true [here](51f2e2f80b/activerecord/lib/active_record/relation/predicate_builder.rb (L58)). This effectively adds a default scope to queries where it shouldn't.
There might be a way to fix this in Active Record but I'd be surprised if nothing else breaks from defining `id` instance and class methods. I think it is simpler to not allow it as a value since it really should be treated as a reserved method.
Minor fixes to the deprecation messages around TestFixtures.fixture_path,
TestFixtures.fixture_path=, and TestFixtures#fixture_path to clarify
which method is deprecated and which should be used instead.
If a has_one association uses a composite primary key, and part of the
composite primary key changes on the owner, these changes need to be
reflected on the belonging object's foreign key.
This was not working previously, because `#_record_changed?` was not
equipped to handle composite primary key associations, so we were not
recognizing that the belonging object's foreign key needed to be updated
when the owner's primary key changed.
Checking whether a record has a foreign key for a composite primary key
association requires us to check whether all parts of the foreign key
are present. Otherwise, the inverse association will not be set.
This commit stops issuing the
"Active Record does not support composite primary key" warning
and allows `ActiveRecord::Base#primary_key` to be derived as an `Array`
`#scope_for_create` for singular associations removes the primary key
from the scope so that we don't assign PK columns when building an
association.
However, removing the primary key from the scope doesn't currently
handle composite primary keys. This commit fixes that.
When find_each/find_in_batches/in_batches are performed on a table with composite primary keys, ascending or descending order can be selected for each key.
```ruby
Person.find_each(order: [:desc, :asc]) do |person|
person.party_all_night!
end
```
This commit handles destroying CPK associations when autosave is set
and the parent association is marked for destruction. It does so
by ensuring that all parts of the foreign key (the parent's CPK)
are set to nil before destroying the parent record.
Anytime an exception is raised from an adapter we now provide a
`connection_pool` along for the application to further debug what went
wrong. This is an important feature when running a multi-database Rails
application.
We chose to provide the `connection_pool` as it has relevant context
like connection, role and shard. We wanted to avoid providing the
`connection` directly as it might accidentally be used after it's
returned to the pool and been handed to another thread.
The `ConnectionAdapters::PoolConfig` would also have been a reasonable
option except it's `:nodoc:`.
While we had hoped to turn prepared statements on for Rails 7.2, the bug
that's preventing us from doing that is still present. See #43005.
Until this bug is fixed we should not be encouraging applications
running mysql to change the `prepared_statements` in the config to
`true`. In addition to this bug being present, Trilogy does not yet
support `prepared_statements` (although work is in progress).
It will be better to implement this deprecation when mysql2 and trilogy
can both handle `prepared_statements` without major bugs.
This commit extends Active Record creation logic to allow for a database
auto-populated attributes to be assigned on object creation.
Given a `Post` model represented by the following schema:
```ruby
create_table :posts, id: false do |t|
t.integer :sequential_number, auto_increment: true
t.string :title, primary_key: true
t.string :ruby_on_rails, default: -> { "concat('R', 'o', 'R')" }
end
```
where `title` is being used as a primary key, the table has an
integer `sequential_number` column populated by a sequence and
`ruby_on_rails` column has a default function - creation of
`Post` records should populate the `sequential_number` and
`ruby_on_rails` attributes:
```ruby
new_post = Post.create(title: 'My first post')
new_post.sequential_number # => 1
new_post.ruby_on_rails # => 'RoR'
```
* At this moment MySQL and SQLite adapters are limited to only one
column being populated and the column must be the `auto_increment`
while PostgreSQL adapter supports any number of auto-populated
columns through `RETURNING` statement.
If an application is using sharding, they may not want to use `default`
as the `default_shard`. Unfortunately Rails expects there to be a shard
named `default` for certain actions internally. This leads to some
errors on boot and the application is left manually setting
`default_shard=` in their model or updating their shards in
`connects_to` to name `shard_one` to `default`. Neither are a great
solution, especially if Rails can do this for you. Changes to Active
Record are:
* Simplify `connects_to` by merging `database` into `shards` kwarg so we
can do a single loop through provided options.
* Set the `self.default_shard` to the first keys in the shards kwarg.
* Add a test for this behavior
* Update existing test that wasn't testing this to use `default`. I
could have left this test but it really messes with connections in the
other tests and since this isn't testing shard behavior specifically, I
updated it to use `default` as the default shard name.
This is a slight change in behavior from existing applications but
arguably this is fixing a bug because without this an application won't
boot. I originally thought that this would require a huge refactoring to
fix but realized that it makes a lot of sense to take the first shard as
they default. They should all have the same schema so we can assume it's
fine to take the first one.
Fixes: #45390
Eileen M. Uchitelle