Commit Graph

8828 Commits

Author SHA1 Message Date
Andrew White
9bdf6a3535
Merge pull request #39464 from rails/make-signed-id-verifier-secret-lazy
Make signed_id_verifier_secret lazily evaluated
2020-05-28 19:18:05 +01:00
Andrew White
9a97b1d62b
Make signed_id_verifier_secret lazily evaluated
The signed id feature introduced in #39313 can cause loading issues
since it may try to generate a key before the secret key base has
been set. To prevent this wrap the secret initialization in a lambda.
2020-05-28 19:00:41 +01:00
Eileen M. Uchitelle
55f519f5b6
Merge pull request #39453 from dylanahsmith/transaction-return-no-raise
activerecord: No warning for return out of transaction block without writes
2020-05-28 08:32:44 -04:00
Ryuta Kamizono
844106efa9 merge doesn't allow to overwrite partially matching nodes
This makes `merge rewhere: true` follow the original `merge`'s behavior.
2020-05-28 08:32:40 +09:00
Ryuta Kamizono
817389d1dd Make remaining migration options to kwargs 2020-05-28 07:51:47 +09:00
Ryuta Kamizono
b9e5859c6f Make index options to kwargs 2020-05-28 07:48:53 +09:00
Dylan Thacker-Smith
4332613b6d activerecord: No warning for return out of transaction block without writes
It doesn't matter if the transaction is rolled back or committed if it
wasn't written to, so we can avoid warning about a breaking change.
2020-05-27 16:49:01 -04:00
Ryuta Kamizono
787b991c94 rewhere allow to overwrite non-attribute nodes
Follow up to #39415.
2020-05-27 09:20:37 +09:00
Ryuta Kamizono
95e130524d Fix flaky assert queries tests
`assert_no_queries` sometimes fails due to counting implict schema info
load queries, most case that is enough to just ignore it.

https://buildkite.com/rails/rails/builds/69598#321bb1bc-ec67-40cd-813f-68dc7809ddde/1444-1452
2020-05-27 08:52:44 +09:00
Ryuta Kamizono
c4682ead52 Fix "warning: instance variable @klass not initialized"
Follow up to #39404.
2020-05-27 00:25:20 +09:00
Ryuta Kamizono
4f95ff7840 Fix preloading for polymorphic association with custom scope
#39378 has changed to use `build_scope` in `join_scopes`, which rely on
`reflection.klass`, but `reflection.klass` is restricted for polymorphic
association, the klass for the association should be passed explicitly.
2020-05-26 04:07:28 +09:00
Ryuta Kamizono
0571da9593
Merge pull request #39415 from kamipo/merge_rewhere_with_non_attrs
`merge(..., rewhere: true)` should have the ability to overwrite non-attribute nodes
2020-05-25 19:11:29 +09:00
Ryuta Kamizono
8e7080e4cc
Merge pull request #39408 from kamipo/remove_limit_on_enum
Remove unused `limit` on `enum` and `set` columns in MySQL
2020-05-25 19:08:47 +09:00
Ryuta Kamizono
e5953db2f3 merge(..., rewhere: true) should have the ability to overwrite non-attribute nodes
Related to #7380 and #7392.

`merge` allows to overwrite non-attribute nodes by #7392, so
`merge(..., rewhere: true)` should also have the same ability, to
migrate from the half-baked current behavior to entirely consistent new
behavior.
2020-05-25 07:21:32 +09:00
sinsoku
26a60c4f29
Allow relations with different SQL comments in or
An error occurs when you pass a relation with SQL comments to the `or` method.

```ruby
class Post
  scope :active, -> { where(active: true).annotate("active posts") }
end

Post.where("created_at > ?", Time.current.beginning_of_month)
  .or(Post.active)
```

In order to work without `ArgumentError`, it changes the `or` method to ignore
SQL comments in the argument.

Ref: https://github.com/rails/rails/pull/38145#discussion_r363024376
2020-05-24 22:43:10 +09:00
Ryuta Kamizono
42914820d9 Do not use object_id on Active Record object directly
Follow up to #38990.
2020-05-24 11:08:24 +09:00
Ryuta Kamizono
86a9b36706 Remove unused limit on enum and set columns in MySQL
Before #36604, `enum` and `set` columns were incorrectly dumped as a
`string` column.

If an `enum` column is defined as `foo enum('apple','orange')`, it was
dumped as `t.string :foo, limit: 6`, the `limit: 6` is seemed to
restrict invalid string longer than `'orange'`.

But now, `enum` and `set` columns are correctly dumped as `enum` and
`set` columns, the limit as longest element is no longer used.
2020-05-24 09:20:55 +09:00
Ryuta Kamizono
859681ec5b Separate primary key column options from table options
Otherwise we cannot handle the same name options for both column and
table (e.g. `:comment`, `:charset`, `:collation`).
2020-05-24 08:15:39 +09:00
Ryuta Kamizono
3c6be5e502 Default engine ENGINE=InnoDB is no longer dumped to make schema more agnostic
5 years ago, I made dumping full table options at #17569, especially to
dump `ENGINE=InnoDB ROW_FORMAT=DYNAMIC` to use utf8mb4 with large key
prefix.

In that time, omitting the default engine `ENGINE=InnoDB` was not useful
since `ROW_FORMAT=DYNAMIC` always remains as long as using utf8mb4 with
large key prefix.

But now, MySQL 5.7.9 has finally changed the default row format to
DYNAMIC, utf8mb4 with large key prefix can be used without dumping the
default engine and the row format explicitly.

So now is a good time to make the default engine is omitted.

Before:

```ruby
create_table "accounts", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_ai_ci", force: :cascade do |t|
end
```

After:

```ruby
create_table "accounts", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
end
```

To entirely omit `:options` option to make schema agnostic, I've added
`:charset` and `:collation` table options to exclude `CHARSET` and
`COLLATE` from `:options`.

Fixes #26209.
Closes #29472.

See also #33608, #33853, and #34742.
2020-05-24 07:50:26 +09:00
David Heinemeier Hansson
fd8fd4ae76
Add delegated type to Active Record (#39341) 2020-05-23 15:14:40 -07:00
player1
277d637277
Take primay_key in count in ActiveRecord::SignedId (#39404)
Co-authored-by: Anton Topchii <player1@infinitevoid.net>
2020-05-23 14:36:06 -07:00
Ryuta Kamizono
35813645b1
Merge pull request #39358 from kamipo/deprecate_group_by_duplicated_fields
Deprecate aggregations with group by duplicated fields
2020-05-24 03:52:29 +09:00
Ryuta Kamizono
2580b83f42 Deprecate aggregations with group by duplicated fields
We've learned that `merge` causes duplicated multiple values easily, so
if we missed to deduplicate the values, it will cause weird behavior
like #38052, #39171.

I've investigated the deduplication for the values, at least that had
existed since Rails 3.0.

bed9179aa1

Aggregations with group by multiple fields was introduced at Rails 3.1,
but we missed the deduplication for the aggregation result, unlike the
generated SQL.

a5cdf0b9eb

While the investigation, I've found that `annotate` is also missed the
deduplication.

I don't suppose this weird behavior is intended for both.

So I'd like to deprecate the duplicated behavior in Rails 6.1, and will
be deduplicated all multiple values in Rails 6.2.

To migrate to Rails 6.2's behavior, use `uniq!(:group)` to deduplicate
group fields.

```ruby
accounts = Account.group(:firm_id)

# duplicated group fields, deprecated.
accounts.merge(accounts.where.not(credit_limit: nil)).sum(:credit_limit)
# => {
#   [1, 1] => 50,
#   [2, 2] => 60
# }

# use `uniq!(:group)` to deduplicate group fields.
accounts.merge(accounts.where.not(credit_limit: nil)).uniq!(:group).sum(:credit_limit)
# => {
#   1 => 50,
#   2 => 60
# }
```
2020-05-24 03:11:53 +09:00
Ryuta Kamizono
7520516605
Merge pull request #39403 from kamipo/merge_should_not_duplicate_same_clauses
Deduplicate same clauses in `merge`
2020-05-24 02:44:54 +09:00
Ryuta Kamizono
5528a79dc7 Deduplicate same clauses in merge
Related to #39328, #39358.

For now, `merge` cannot override non-equality clauses, so non-equality
clauses will easily be duplicated by `merge`.

This deduplicates redundant same clauses in `merge`.
2020-05-24 02:14:41 +09:00
Ryuta Kamizono
6a617cc61e Fix through association with source/through scope which has joins
If source/through scope references other tables in where/order, we
should explicitly maintain joins in the scope, otherwise association
queries will fail with referenced unknown column.

Fixes #33525.
2020-05-22 04:05:27 +09:00
Ryuta Kamizono
489df7eb28 Fix through association to respect source scope for includes/preload
`reflection.scope` is not aware of all source scopes if the association
is through association.

It should use `reflection.join_scopes` for that.

Fixes #39376.
2020-05-21 07:40:01 +09:00
Ryuta Kamizono
c50170d213 loaded should be aliased to loaded? in collection proxy
Fixes #39088.
2020-05-20 22:25:29 +09:00
Aaron Lipman
cb554457a9 Resolve conflict between counter cache and optimistic locking
Bump an Active Record instance's lock version after updating its counter
cache. This avoids raising an unnecessary ActiveRecord::StaleObjectError
upon subsequent transactions by maintaining parity with the
corresponding database record's lock_version column.
2020-05-19 20:39:41 -04:00
eileencodes
5b5da30bc0
Fix index options for if_not_exists/if_exists
The `index_exists?` method wasn't very specific so when we added the
`if_not_exists` to `add_index` and `if_exists` to `remove_index` there
were a few cases where behavior was unexpected.

For `add_index` if you added a named index and then added a second index
with the same columns, but a different name, that index would not get
added because `index_exists` was looking only at column named and not at
the exact index name. We fixed `add_index` by moving the `index_exists`
check below `add_index_options` and pass `name` directly to
`index_exists` if there is a `if_not_exists` option.

For `remove_index` if you added a named index and then tried to remove
it with a nil column and a explicit name the index would not get removed
because `index_exists` saw a nil column. We fixed this by only doing the
column check in `index_exists` if `column` is present.

Co-authored-by: John Crepezzi <john.crepezzi@gmail.com>
2020-05-19 16:19:22 -04:00
Aaron Patterson
e3c7ba4612
Merge pull request #39338 from p8/did-you-mean-for-has-many-through
Add DidYouMean for HasManyThroughAssociationNotFoundError
2020-05-19 08:37:01 -07:00
Petrik
9082364a25 Add DidYouMean for HasManyThroughAssociationNotFoundError
If a has_many :through association isn't found we can suggest similar associations:

```
class Author
  has_many :categorizations, -> { }
  has_many :categories, through: :categorizations
  has_many :categorized_posts, through: :categorizations, source: :post
  has_many :category_post_comments, through: :categories, source: :post_comments

  has_many :nothings, through: :kateggorisatons, class_name: "Category"
end

Author.first.nothings

Could not find the association :kateggorisatons in model Author
Did you mean?  categorizations
               categories
               categorized_posts
               category_post_comments
```
2020-05-19 08:27:08 +02:00
Ryuta Kamizono
4774fa0d26 Test multiple values with blank value 2020-05-19 04:57:00 +09:00
Ryuta Kamizono
cdbce035a5
Merge pull request #39330 from kamipo/fix_merge_not_in
Fix merging NOT IN clause to behave the same as before
2020-05-18 21:48:36 +09:00
Ryuta Kamizono
13a3f62ae8 Fix merging NOT IN clause to behave the same as before
`HomogeneousIn` has changed merging behavior for NOT IN clause from
before. This changes `equality?` to return true only if `type == :in` to
restore the original behavior.
2020-05-18 15:56:44 +09:00
Ryuta Kamizono
fa63e5f03e
Merge pull request #39219 from kamipo/bind_param
Should not substitute binds when `prepared_statements: true`
2020-05-18 15:20:00 +09:00
Ryuta Kamizono
f6fb3271d4 Support merging option :rewhere to allow mergee side condition to be replaced exactly
Related #39236.

`relation.merge` method sometimes replaces mergee side condition, but
sometimes maintain both conditions unless `relation.rewhere` is used.

It is very hard to predict merging result whether mergee side condition
will be replaced or not.

One existing way is to use `relation.rewhere` for merger side relation,
but it is also hard to predict a relation will be used for `merge` in
advance, except one-time relation for `merge`.

To address that issue, I propose to support merging option `:rewhere`,
to allow mergee side condition to be replaced exactly.

That option will allow non-`rewhere` relation behaves as `rewhere`d
relation.

```ruby
david_and_mary = Author.where(id: david.id..mary.id)

# both conflict conditions exists
david_and_mary.merge(Author.where(id: bob)) # => []

# mergee side condition is replaced by rewhere
david_and_mary.merge(Author.rewhere(id: bob)) # => [bob]

# mergee side condition is replaced by rewhere option
david_and_mary.merge(Author.where(id: bob), rewhere: true) # => [bob]
```
2020-05-18 14:36:42 +09:00
Ryuta Kamizono
a095916df1
Merge pull request #39321 from kamipo/fix_update_with_dirty_locking_column
Fix update with dirty locking column to not match latest object accidentally
2020-05-18 11:46:04 +09:00
Ryuta Kamizono
6bd2bb7526 Fix rename column in bulk alter table for PostgreSQL
PostgreSQL doesn't support rename column operation in bulk alter table.

Fixes #39322.
2020-05-18 11:14:29 +09:00
Ryuta Kamizono
fba016c7a3 Fix update with dirty locking column to not match latest object accidentally
Related #32163.

We should not identify an object by dirty value. If do so, accidentally
matches latest object even though it is a stale object.
2020-05-18 09:46:09 +09:00
Ryuta Kamizono
c360644ed9 Do not need explicit handler for InfixOperation's subclasses
Unless explicitly intend to behave it differently from the superclass.
2020-05-18 08:18:55 +09:00
Aaron Patterson
b027288823
Merge pull request #39318 from p8/did-you-mean-for-associations
Use DidYouMean for AssociationNotFoundError
2020-05-17 15:24:13 -07:00
Petrik
3bc7756036 Use DidYouMean for AssociationNotFoundError
If an association isn't found we can suggest matching associations:

```
Post.all.merge!(includes: :monkeys).find(6)

Association named 'monkeys' was not found on Post; perhaps you misspelled it?
Did you mean?  funky_tags
               comments
               images
               skimmers
```
2020-05-17 21:32:19 +02:00
David Heinemeier Hansson
1a3dc42c17
Add signed ids to Active Record (#39313)
Add support for finding records based on signed ids, which are tamper-proof, verified ids that can be set to expire and scoped with a purpose. This is particularly useful for things like password reset or email verification, where you want the bearer of the signed id to be able to interact with the underlying record, but usually only within a certain time period.
2020-05-17 11:19:37 -07:00
Ryuta Kamizono
77f7b2df3a Remove sync_with_transaction_state to simplify code base
This also removes the `if @transaction_state&.finalized?` guard which is
harder to understand optimization introduced at #36049. The guard is
faster enough though, but removing that will make attribute access about
2% ~ 4% faster, and will make code base to ease to maintain.

`sync_with_transaction_state` was introduced at #9068 to address memory
bloat when creating lots of AR objects inside a transaction.

I've found #18638 the same design of this to address memory bloat, but
this differs from #18638 in that it will allocate one `WeakMap` object
only when explicit transaction, no extra allocation for implicit
transaction.

Executable script to reproduce memory bloat:

https://gist.github.com/kamipo/36d869fff81cf878658adc26ee38ea1b
https://github.com/rails/rails/issues/15549#issuecomment-46035848

I can see no memory concern with this.

Co-authored-by: Arthur Neves <arthurnn@gmail.com>
2020-05-17 20:57:32 +09:00
Matthew Draper
58bed97226
Merge pull request #39235 from hayesr/cte_alias_predication
Allow the construction of a CTE from an Arel::Table
2020-05-17 04:13:50 +09:30
Ryuta Kamizono
dec12b5cb7
Merge pull request #39305 from kamipo/fix_eager_load_with_arel_joins
Fix eager load with Arel joins to maintain the original joins order
2020-05-16 18:42:54 +09:00
Ryuta Kamizono
af71a82265
Merge pull request #39297 from kamipo/fix_group_by_order_and_limit_offset
Fix group by count with eager loading + order + limit/offset
2020-05-16 18:42:26 +09:00
Ryuta Kamizono
8f365c5d04 Fix eager load with Arel joins to maintain the original joins order
#38354 is caused by #36304, to fix invalid joins order for through
associations.

Actually passing Arel joins to `joins` is not officially supported
unlike string joins, and also Arel joins could be easily converted to
string joins by `to_sql`. But I'd not intend to break existing apps
without deprecation cycle, so I've changed to mark only implicit joins
as leading joins, to maintain the original joins order for user supplied
Arel joins.

Fixes #38354.
2020-05-16 10:17:11 +09:00
Ryuta Kamizono
5812feffe3 Fix group by count with eager loading + order + limit/offset
`count` is too complex feature in Active Record, it is heavily mangling
select values, so it easily hit the ORDER BY with SELECT DISTINCT
limitation.

35bf86fe83/activerecord/lib/active_record/relation/calculations.rb (L133-L140)
35bf86fe83/activerecord/lib/active_record/relation/calculations.rb (L253-L264)

But at least in the case of group by, select values are always to be
aggregated value and group values, so meaningful order values are
originally limited. So in that case, remaining order values should
be safe as long as meaningful order values are set by people.

Fixes #38936.
2020-05-15 17:16:59 +09:00