| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
|
|
|
| |
Since d1a74c1e012ed96f7179e53b9190b7da0a369e11, Active Record requires
SQLite version 3.8.0 or greater, so savepoints and partial indexes are
always available.
That commit also added a runtime version check, so we can remove the
minimum version requirement from the internal adapter documentation.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
transaction
Currently, `committed!`/`rolledback!` will only be attempted for the
first enrolled record in the transaction, that will cause some
problematic behaviors.
The first one problem, `clear_transaction_record_state` won't be called
even if the transaction is finalized except the first enrolled record.
This means that de-duplicated records in the transaction won't refer
latest state (e.g. won't happen rolling back record state).
The second one problem, the enrolled order is not always the same as the
order in which the actions actually happened, the first enrolled record
may succeed no actions (e.g. `destroy` has already succeeded on another
record during `before_destroy`), it will lose to fire any transactional
callbacks.
To avoid both problems, we should attempt `committed!`/`rolledback!` to
all enrolled records in the transaction.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
If the same id's records are saved and/or destroyed in the transaction,
commit callbackes will only run for the first enrolled record.
https://github.com/rails/rails/blob/a023e2180093ebc517a642aaf21f3c7241c67657/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb#L115-L119
The regression #36132 is caused due to #35920 changed the enrollment
order that the first action's record will be enrolled to last in the
transaction.
We could not change the the enrollment order as long as someone depends
on the enrollment order.
Fixes #36132.
|
|
|
|
| |
Follow up to b1458218c95d85c4ce911dd3e99da5ae7cf7aeee.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I've realized that `user.id` is 20% slower than `user.name` in the
benchmark (https://github.com/rails/rails/pull/35987#issuecomment-483882480).
The reason that performance difference is that `self.class.primary_key`
method call is a bit slow.
Avoiding that method call will make almost attribute access faster and
`user.id` will be completely the same performance with `user.name`.
Before (02b5b8cb):
```
Warming up --------------------------------------
user.id 140.535k i/100ms
user['id'] 96.549k i/100ms
user.name 158.110k i/100ms
user['name'] 94.507k i/100ms
user.changed? 19.003k i/100ms
user.saved_changes? 25.404k i/100ms
Calculating -------------------------------------
user.id 2.231M (± 0.9%) i/s - 11.243M in 5.040066s
user['id'] 1.310M (± 1.3%) i/s - 6.565M in 5.012607s
user.name 2.683M (± 1.2%) i/s - 13.439M in 5.009392s
user['name'] 1.322M (± 0.9%) i/s - 6.615M in 5.003239s
user.changed? 201.999k (±10.9%) i/s - 1.007M in 5.091195s
user.saved_changes? 258.214k (±17.1%) i/s - 1.245M in 5.007421s
```
After (this change):
```
Warming up --------------------------------------
user.id 158.364k i/100ms
user['id'] 106.412k i/100ms
user.name 158.644k i/100ms
user['name'] 107.518k i/100ms
user.changed? 19.082k i/100ms
user.saved_changes? 24.886k i/100ms
Calculating -------------------------------------
user.id 2.768M (± 1.1%) i/s - 13.936M in 5.034957s
user['id'] 1.507M (± 2.1%) i/s - 7.555M in 5.017211s
user.name 2.727M (± 1.5%) i/s - 13.643M in 5.004766s
user['name'] 1.521M (± 1.3%) i/s - 7.634M in 5.018321s
user.changed? 200.865k (±11.1%) i/s - 992.264k in 5.044868s
user.saved_changes? 269.652k (±10.5%) i/s - 1.344M in 5.077972s
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Method call in Ruby is a bit slow.
This makes attribute access 10% faster by avoiding method call
(`sync_with_transaction_state`).
Before (96cf7e0e):
```
Warming up --------------------------------------
user.id 131.291k i/100ms
user['id'] 91.786k i/100ms
user.name 151.605k i/100ms
user['name'] 92.664k i/100ms
user.changed? 17.772k i/100ms
user.saved_changes? 23.909k i/100ms
Calculating -------------------------------------
user.id 1.988M (± 7.0%) i/s - 9.978M in 5.051474s
user['id'] 1.155M (± 5.8%) i/s - 5.783M in 5.022672s
user.name 2.450M (± 4.3%) i/s - 12.280M in 5.021234s
user['name'] 1.263M (± 2.1%) i/s - 6.394M in 5.066638s
user.changed? 175.070k (±13.3%) i/s - 853.056k in 5.011555s
user.saved_changes? 259.114k (±11.8%) i/s - 1.267M in 5.001260s
```
After (this change):
```
Warming up --------------------------------------
user.id 137.625k i/100ms
user['id'] 96.054k i/100ms
user.name 156.379k i/100ms
user['name'] 94.795k i/100ms
user.changed? 18.172k i/100ms
user.saved_changes? 24.337k i/100ms
Calculating -------------------------------------
user.id 2.201M (± 0.5%) i/s - 11.010M in 5.002955s
user['id'] 1.320M (± 1.0%) i/s - 6.628M in 5.021293s
user.name 2.677M (± 1.6%) i/s - 13.449M in 5.024399s
user['name'] 1.314M (± 1.8%) i/s - 6.636M in 5.051444s
user.changed? 190.588k (±11.1%) i/s - 944.944k in 5.065848s
user.saved_changes? 262.782k (±12.1%) i/s - 1.290M in 5.028080s
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently the rollback only restores primary key value, `new_record?`,
`destroyed?`, and `frozen?`. Since the `save` clears current dirty
attribute states, retrying save after rollback will causes no change
saved if partial writes is enabled (by default).
This makes `remember_transaction_record_state` remembers original values
then restores dirty attribute states after rollback.
Fixes #15018.
Fixes #30167.
Fixes #33868.
Fixes #33443.
Closes #33444.
Closes #34504.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since 8180c39, remaining transaction state is cleared in
`force_clear_transaction_record_state` to less work
`sync_with_transaction_state`. But it caused a race condition that
`@transaction_state` would be cleared by other threads if the state is
finalized.
To work as before, snapshot `@transaction_state` to local variable not
to refer `@transaction_state` directly.
Fixes #35983.
|
|
|
|
|
|
|
| |
All transaction states (:committed, :fully_committed, :rolledback,
:fully_rolledback) are mutually exclusive.
And also, `force_clear_transaction_record_state` should clear
`@transaction_state` as well.
|
|
|
|
| |
`set_transaction_state`
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
`add_to_transaction` was added at da840d1, but it should not be called
by except internal, since `remember_transaction_record_state` should be
called only once before saving.
And also, currently `add_to_transaction` doesn't always add the record
to transaction since da8de91, that is the reason hard to use that even
in internal.
Even if `add_to_transaction` ensure to add the record to transaction,
that is an internal concern, people don't need to explicitly call
`add_to_transaction`.
|
|
|
|
|
|
|
|
|
|
|
| |
Regardless of a record isn't saved (e.g. validation is failed),
`after_commit` / `after_rollback` callbacks are invoked for now.
To fix the issue, this adds a record to the current transaction only
when a record is actually saved.
Fixes #29747.
Closes #29833.
|
|
|
|
| |
:create, :update ]`. (#35804)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The test added in 12b0b26df7560ab5199ba830586864085441508f passes even
without this code since 9b8c7796a9c2048208aa843ad3dc477dffa8bdee, as the
call to `id` in `remember_transaction_record_state` now triggers a
`sync_with_transaction_state` which discards the leftover state from the
previous transaction.
This issue had already been fixed for `save!`, `destroy` and `touch` in
caae79a385ce112245262a17414bcd96bea013c2, but continued to affect `save`
because the call to `rollback_active_record_state!` in that method would
increment the transaction level before `add_to_transaction` could clear
it, preventing the fix from working correctly.
As `rollback_active_record_state!` was removed entirely in
48007d5390db47fc1223f57c8e7ab3ebb7c3a3d7, this code is no longer needed.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Since a record is already persisted in `after_create_commit`, so `save`
should invoke only `after_update_commit`.
This bug is caused by depending on `@_start_transaction_state` for
rollback to consider whether it was `new_record` before being committed.
If after commit callbacks caused another commit, the state before
last commit is no longer `new_record`.
Fixes #32831.
Closes #18367.
Closes #31106.
|
|
|
|
| |
Fixes #32806.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
After a real (non-savepoint) transaction has committed or rolled back,
the original persistence-related state for all records modified in that
transaction is discarded or restored, respectively.
When the model has transactional callbacks, this happens synchronously
in the `committed!` or `rolled_back!` methods; otherwise, it happens
lazily the next time the record's persistence-related state is accessed.
The synchronous code path always finalizes the state of the record, but
the lazy code path only pops one "level" from the transaction counter,
assuming it will always reach zero immediately after a real transaction.
As the test cases included here demonstrate, that isn't always the case.
By using the same logic as the synchronous code path, we ensure that the
record's state is always updated after a real transaction has finished.
|
|\
| |
| | |
Remove ActiveRecord::Transactions#rollback_active_record_state!
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
`rollback_active_record_state!` was removed from `save!` but not `save`
in da840d13da865331297d5287391231b1ed39721b. I believe that leaving it
in `save` was a mistake, since that commit was intended to move the
rollback logic from the `save`/`save!` call to the transaction stack.
As of 67d8bb963d5d51fc644d6b1ca20164efb4cee6d7 the record's original
state is lazily restored the first time it's accessed after the
transaction, instead of when a rollback occurs. This means that the call
to `restore_transaction_record_state` here has no effect: the record's
transaction level is incremented twice (in rollback_active_record_state!
and `with_transaction_returning_status`), isn't decremented again until
the the `ensure` block runs, and won't hit zero until the next time
`sync_with_transaction_state` is called.
|
|/
|
|
|
|
|
| |
If an `ActiveRecord::Rollback` error was raised by a persistence method
(e.g. in an `after_save` callback), this logic would potentially discard
the original state of the record from before the transaction, preventing
it from being restored later when the transaction was rolled back.
|
|
|
|
| |
Commit callbacks are intentionally disabled when errors occur when calling the callback chain in order to reset the internal record state. However, the implicit order of operations on the logic for checking if callbacks are disabled is wrong. The result is that callbacks can be unexpectedly when errors occur in transactions.
|
| |
|
| |
|
|
|
|
| |
[ci skip]
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
refs: https://github.com/rails/rails/pull/30161
```
$ echo "+@size+" | rdoc --pipe
<p>+@size+</p>
$ echo "<tt>@size</tt>" | rdoc --pipe
<p><code>@size</code></p>
```
[ci skip]
|
|\
| |
| | |
Sync transaction state when accessing primary key
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
If a record is modified inside a transaction, it must check the outcome
of that transaction before accessing any state which would no longer be
valid if it was rolled back.
For example, consider a new record that was saved inside a transaction
which was later rolled back: it should be restored to its previous state
so that saving it again inserts a new row into the database instead of
trying to update a row that no longer exists.
The `id` and `id=` methods defined on the PrimaryKey module implement
this correctly, but when a model uses a custom primary key, the reader
and writer methods for that attribute must check the transaction state
too. The `read_attribute` and `write_attribute` methods also need to
check the transaction state when accessing the primary key.
|
|/ |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Let's say you have a nested transaction and both records are saved.
Before the outer transaction closes, a rollback is performed. Previously
the record in the outer transaction would get marked as not persisted
but the inner transaction would get persisted.
```ruby
Post.transaction do
post_one.save # will get rolled back
Post.transaction(requires_new: true) do
post_two.save # incorrectly remains marked as persisted
end
raise ActiveRecord::Rollback
end
```
To fix this the PR changes transaction handling to have the child
transaction ask the parent how the records should be marked. When
there are child transactions, it will always be a SavpointTransaction
because the stack isn't empty. From there we pass the parent_transaction
to the child SavepointTransaction where we add the children to the parent
so the parent can mark the inner transaction as rolledback and thus mark
the record as not persisted.
`update_attributes_from_transaction_state` uses the `completed?` check to
correctly mark all the transactions as rolledback and the inner record as
not persisted.
```ruby
Post.transaction do
post_one.save # will get rolled back
Post.transaction(requires_new: true) do
post_two.save # with new behavior, correctly marked as not persisted
on rollback
end
raise ActiveRecord::Rollback
end
```
Fixes #29320
|
|
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| |
|
|
|
|
|
| |
The old top level classes PGconn, PGresult and PGError were deprecated
since pg-0.13.0: https://github.com/ged/ruby-pg/blob/master/History.rdoc#v0130-2012-02-09-michael-granger-gedfaeriemudorg
|
| |
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
| |
Race conditions can occur when an ActiveRecord is destroyed
twice or destroyed and updated. The callbacks should only be
triggered once, similar to a SQL database trigger.
|
|
|
|
|
|
| |
All indentation was normalized by rubocop auto-correct at 80e66cc4d90bf8c15d1a5f6e3152e90147f00772.
But comments was still kept absolute position. This commit aligns
comments with method definitions for consistency.
|
| |
|
|
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
| |
|
|
|
|
|
|
|
|
| |
Just noticed this on the [edge API].
[ci skip]
[edge API]: http://edgeapi.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Those are actually shortcuts for `after_commit`.
Before:
after_commit :add_to_index_later, on: :create
after_commit :update_in_index_later, on: :update
after_commit :remove_from_index_later, on: :destroy
After:
after_create_commit :add_to_index_later
after_update_commit :update_in_index_later
after_destroy_commit :remove_from_index_later
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The focus of this change is to make the API more accessible.
References to method and classes should be linked to make it easy to
navigate around.
This patch makes exzessiv use of `rdoc-ref:` to provide more readable
docs. This makes it possible to document `ActiveRecord::Base#save` even
though the method is within a separate module
`ActiveRecord::Persistence`. The goal here is to bring the API closer to
the actual code that you would write.
This commit only deals with Active Record. The other gems will be
updated accordingly but in different commits. The pass through Active
Record is not completely finished yet. A follow up commit will change
the spots I haven't yet had the time to update.
/cc @fxn
|
|
|
|
|
|
| |
Rails 4.2.3 AS::Callbacks will not halt chain if `false` is returned.
That is the behavior of specific callbacks like AR::Callbacks and
AM::Callbacks.
|
|
|
|
|
| |
skip]
Bumps from `5.6` to `5.7`
|