| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
to support Oracle database support identity data type
Oracle database does not support `INSERT .. DEFAULT VALUES`
then every insert statement needs at least one column name specified.
When `prefetch_primary_key?` returns `true` insert statement
always have the primary key name since the primary key value is selected
from the associated sequence. However, supporting identity data type
will make `prefetch_primary_key?` returns `false`
then no primary key column name added.
As a result, `empty_insert_statement_value` raises `NotImplementedError`
To address this error `empty_insert_statement_value` can take
one argument `primary_key` to generate insert statement like this.
`INSERT INTO "POSTS" ("ID") VALUES(DEFAULT)`
It needs arity change for the public method but no actual behavior changes for the bundled adapters.
Oracle enhanced adapter `empty_insert_statement_value` implementation will be like this:
```
def empty_insert_statement_value(primary_key)
raise NotImplementedError unless primary_key
"(#{quote_column_name(primary_key)}) VALUES(DEFAULT)"
end
```
[Raise NotImplementedError when using empty_insert_statement_value with Oracle](https://github.com/rails/rails/pull/28029)
[Add support for INSERT .. DEFAULT VALUES](https://community.oracle.com/ideas/13845)
|
|
|
|
|
|
|
| |
The multiple assignments was caused by 37a1dfa due to lost the `to_s`
normalization for given names.
Fixes #32323.
|
|
|
|
| |
This reverts commit a19e91f0fab13cca61acdb1f33e27be2323b9786.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
locking is enabled
This issue is caused by `@_trigger_update_callback` won't be updated due
to `_update_record` in `Locking::Optimistic` doesn't call `super` when
optimistic locking is enabled.
Now optimistic locking concern when updating is supported by
`_update_row` low level API, therefore overriding `_update_record` is no
longer necessary.
Removing the method just fix the issue.
Closes #29096.
Closes #29321.
Closes #30823.
|
|
|
|
| |
`Persistence` module
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
Currently primary key value can not be updated if a record has a locking
column because of `_update_record` in `Locking::Optimistic` doesn't
respect `id_in_database` as primary key value unlike in `Persistence`.
And also, if a record has dirty primary key value, it may destroy any
other record by the lock version of dirty record itself.
When updating/destroying persisted records, it should identify
themselves by `id_in_database`, not by dirty primary key value.
|
|
|
|
|
| |
This removes `|| id` which were added in #9963 and #23887 since it is no
longer necessary.
|
|
|
|
| |
See 948b931925febac3c965ab13470065ced68f7b53 for context
|
|
|
|
|
|
|
|
|
|
|
| |
Currently the place where we limit what gets sent to the database is in
the implementation for `partial_writes`. We should also be restricting
it to column names when partial writes are turned off.
Note that we're using `&` instead of just defaulting to
`self.class.column_names`, as the instance version of `attribute_names`
does not include attributes which are uninitialized (were not included
in the select clause)
|
|
|
|
| |
Closes #31998
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes issue described in #30399: A default value on the
inheritance column prevented `child.becomes(Parent)` to return
an instance of `Parent` as expected, instead it returns an instance
of the default subclass.
The change was introduced by #17169 and it was meant to affect
initialization, alone. Where `Parent.new` is expected to return
an instance of the default subclass.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
#30985 caused `object.save` performance regression since calling
`changes` in `changes_applied` is very slow.
We don't need to call the expensive method in `changes_applied` as long
as `@attributes` is tracked by mutation tracker.
https://gist.github.com/kamipo/1a9f4f3891803b914fc72ede98268aa2
Before:
```
Warming up --------------------------------------
create_string_columns
73.000 i/100ms
Calculating -------------------------------------
create_string_columns
722.256 (± 5.8%) i/s - 3.650k in 5.073031s
```
After:
```
Warming up --------------------------------------
create_string_columns
96.000 i/100ms
Calculating -------------------------------------
create_string_columns
950.224 (± 7.7%) i/s - 4.800k in 5.084837s
```
|
|
|
|
|
|
|
|
|
|
| |
Currently `object.save` will unfreeze the object, due to
`changes_applied` replaces frozen `@attributes` to new `@attributes`.
Since originally destroyed objects are not allowed to be mutated, `save`
and `save!` should not return success in that case.
Fixes #28563.
|
|
|
|
|
|
|
|
| |
update_attribute previously stopped execution, before saving and before
running callbacks, if the record's attributes hadn't changed. [The
documentation](http://api.rubyonrails.org/classes/ActiveRecord/Persistence.html#method-i-update_attribute)
says that "Callbacks are invoked", which was not happening if the
persisted attributes hadn't changed.
|
|
|
|
|
| |
making changes (#31306)
It makes more sense than ignoring invalid IDs.
|
|
|
|
|
|
|
|
|
|
|
| |
In 35836019, class level `update` and `destroy` suppressed
`RecordNotFound` to ensure returning affected objects. But
`RecordNotFound` is a common exception caught by a `rescue_from`
handler. So changing the behavior when a typical `params[:id]` is passed
has a compatibility problem. The previous behavior should not be
changed.
Fixes #31301.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
`_update_record` (#29999)
Currently `_create_record` and `_update_record` in `Persistence` are
creating extra `unscoped` and calling `build_arel` in the relation. But
`compile_insert` and `compile_update` can be done without those
expensive operation for `SelectManager` creation. So I moved the
implementation to `Persistence` to avoid creating extra relation and
refactored to avoid calling `build_arel`.
https://gist.github.com/kamipo/8ed73d760112cfa5f6263c9413633419
Before:
```
Warming up --------------------------------------
_update_record 150.000 i/100ms
Calculating -------------------------------------
_update_record 1.548k (±12.3%) i/s - 7.650k in 5.042603s
```
After:
```
Warming up --------------------------------------
_update_record 201.000 i/100ms
Calculating -------------------------------------
_update_record 2.002k (±12.8%) i/s - 9.849k in 5.027681s
```
30% faster for STI classes.
|
|\
| |
| | |
activerecord: Remove a redundant mutation tracker
|
| |
| |
| |
| |
| |
| |
| | |
The extra mutation tracker was needed in Rails 5.1 to preserve the
old behaviour of `changes`, but now there is no difference
between `changes` and `changes_to_save`, so `@mutation_tracker`
can be removed.
|
| | |
|
| |
| |
| |
| |
| |
| |
| | |
Class level `update` and `destroy` are using `find` in the internal, so
it will raise `RecordNotFound` if given ids cannot find an object even
though the method already affect (update or destroy) to any objects.
These methods should return affected objects even in that case.
|
|/
|
|
|
|
|
|
|
|
| |
`Persistence::ClassMethods`
The docs are obviously for class level `update`, `destroy`, and
`delete`. It should be placed in `Persistence::ClassMethods` rather than
`Relation`. And also, these methods are not dependent on relation. So it
is not needed to delegate to `all` (plus, `klass.find` is faster than
`relation.find`).
|
| |
|
|
|
|
|
|
|
| |
Anywhere that we're doing `self.class.unscoped.where(primary_key => id)`
is somewhere that someone may want to extend. Even if this method isn't
public API yet, this will make it easier for us to eventually expose an
API around this. Plus, duplicated code makes me sad.
|
| |
|
|
|
|
|
| |
`self.class.delete` is delegated to `all` and `all` is affected by
scoping. It should use `unscoped` to not be affected by that.
|
|\
| |
| | |
Improve the performance of writing attributes
|
| |
| |
| |
| |
| |
| |
| |
| | |
This name more accurately describes what the method does, and also
disambiguates it from `_write_attribute`, which ignores aliases.
We can also make the method private, since it's not public API and only
called from one place - `update_columns` - without an explicit receiver.
|
| |
| |
| |
| |
| | |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| | |
|
|/
|
|
|
|
| |
`ActiveRecord::Persistence#touch` does not work well when optimistic
locking enabled and `locking_column`, without default value, is null in
the database.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
association saved in the callback
Related #18155, #26661, 268a5bb, #27434, #27442, and #28599.
Originally #18155 was introduced for preventing double insertion caused
by the after save callback. But it was caused the before save issue
(#26661). 268a5bb fixed #26661, but it was caused the performance
regression (#27434). #27442 added new record to `target` before calling
callbacks for fixing #27434. But it was caused double firing before save
callback (#28599). We cannot add new object to `target` before saving
the object.
This is improving #18155 to only track callbacks after `save`.
Fixes #28599.
|
|
|
|
|
| |
I can never remember if it returns `self` or `true` (seems to be `true`)
[ci skip]
|
|
|
|
|
| |
Supports the `touch` option from update_counters.
The default behavior is not to update timestamp columns.
|
|\
| |
| | |
Update increment! documentation [ci skip]
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
The `increment!` and `decrement!` methods were recently reimplemented to
make them safe to call from multiple connections concurrently. This
changed their behaviour in a few ways.
Previously they used `update_attribute`, which calls the attribute
setter method, runs callbacks, and touches the record. Now they behave
more like `update_column`, writing the update to the database directly
and bypassing all of those steps.
|
| |
| |
| |
| |
| |
| | |
Closes #27555.
[ Ben A. Morgan & Kasper Timm Hansen ]
|
| |
| |
| |
| |
| |
| | |
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.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
callbacks
We pretty frequently get bug reports that "dirty is broken inside of
after callbacks". Intuitively they are correct. You'd expect
`Model.after_save { puts changed? }; model.save` to do the same thing as
`model.save; puts model.changed?`, but it does not.
However, changing this goes much farther than just making the behavior
more intuitive. There are a _ton_ of places inside of AR that can be
drastically simplified with this change. Specifically, autosave
associations, timestamps, touch, counter cache, and just about anything
else in AR that works with callbacks have code to try to avoid "double
save" bugs which we will be able to flat out remove with this change.
We introduce two new sets of methods, both with names that are meant to
be more explicit than dirty. The first set maintains the old behavior,
and their names are meant to center that they are about changes that
occurred during the save that just happened. They are equivalent to
`previous_changes` when called outside of after callbacks, or once the
deprecation cycle moves.
The second set is the new behavior. Their names imply that they are
talking about changes from the database representation. The fact that
this is what we really care about became clear when looking at
`BelongsTo.touch_record` when tests were failing. I'm unsure that this
set of methods should be in the public API. Outside of after callbacks,
they are equivalent to the existing methods on dirty.
Dirty itself is not deprecated, nor are the methods inside of it. They
will only emit the warning when called inside of after callbacks. The
scope of this breakage is pretty large, but the migration path is
simple. Given how much this can improve our codebase, and considering
that it makes our API more intuitive, I think it's worth doing.
|
| |
| |
| | |
Noticed that the `validate: false` option for `ActiveRecord::Persistence#save` and `#save!` were not formatted as code like the other examples in the documentation.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
- If the attribute is not changed, then update_attribute does not run
SQL query, this effectively means that no change was made to the
attribute.
- This change was made in https://github.com/rails/rails/commit/0fcd4cf5
to avoid a SQL call.
- But the change resulted into `nil` being returned when there was no
change in the attribute value.
- This commit corrects the behavior to return true if there is no change
in attribute value. This is same as previous behavior of Rails 4.2
plus benefit of no additional SQL call.
- Fixes #26593.
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Without this the changes to the lock version column will stick around
even after `touch` returns.
Before:
model.touch
model.changes
# => {"lock_version"=>[0, "1"]}
After:
model.touch
model.changes
# {}
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
If a parent association was accessed in an `after_find` or
`after_initialize` callback, it would always end up loading the
association, and then immediately overwriting the association we just
loaded. If this occurred in a way that the parent's `current_scope` was
set to eager load the child, this would result in an infinite loop and
eventually overflow the stack.
For records that are created with `.new`, we have a mechanism to
perform an action before the callbacks are run. I've introduced the same
code path for records created with `instantiate`, and updated all code
which sets inverse instances on newly loaded associations to use this
block instead.
Fixes #26320.
|
|/ |
|
|
|
|
|
| |
The current code base is not uniform. After some discussion,
we have chosen to go with double quotes by default.
|
| |
|
|
|
|
|
|
| |
The current error message only indicates that a touch can fail because the record is new. In practice, we saw cases where touches were failing because the record had been destroyed. `persisted?` checks `new_record?` *and* `destroyed?`. It was confusing to get a message about a new record when in reality we were destroying records.
I also included a helpful tip for users to consider using `persisted?`, `new_record?`, or `destroyed?` before touching.
|