aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/test/cases/relations_test.rb
Commit message (Collapse)AuthorAgeFilesLines
* Use `try` only when we're unsure if the receiver would respond_to the methodAkira Matsuda2019-08-011-1/+1
|
* `length(title)` is a safe SQL string since #36448Ryuta Kamizono2019-06-261-2/+2
|
* PostgreSQL: Fix GROUP BY with ORDER BY virtual count attributeRyuta Kamizono2019-06-171-2/+2
| | | | | | | | | | | | | GROUP BY with virtual count attribute is invalid for almost all databases, but it is valid for PostgreSQL, and it had worked until Rails 5.2.2, so it is a regression for Rails 5.2.3 (caused by 311f001). I can't find perfectly solution for fixing this for now, but I would not like to break existing apps, so I decided to allow referencing virtual count attribute in ORDER BY clause when GROUP BY aggrigation (it partly revert the effect of 311f001) to fix the regression #36022. Fixes #36022.
* Allow column name with function (e.g. `length(title)`) as safe SQL stringRyuta Kamizono2019-06-101-3/+3
| | | | | | | | | | | | | | | | Currently, almost all "Dangerous query method" warnings are false alarm. As long as almost all the warnings are false alarm, developers think "Let's ignore the warnings by using `Arel.sql()`, it actually is false alarm in practice.", so I think we should effort to reduce false alarm in order to make the warnings valuable. This allows column name with function (e.g. `length(title)`) as safe SQL string, which is very common false alarm pattern, even in the our codebase. Related 6c82b6c99, 6607ecb2a, #36420. Fixes #32995.
* NULLS { FIRST | LAST } is safe SQL string since ↵Ryuta Kamizono2019-06-071-6/+6
| | | | | | 6c82b6c99d86f37e61f935fb342cccd725d6c7d4 There is no need to be wrapped by `Arel.sql()`.
* Allow quoted identifier string as safe SQL stringRyuta Kamizono2019-06-061-2/+2
| | | | | | | | | | | | | Currently `posts.title` is regarded as a safe SQL string, but `"posts"."title"` (it is a result of `quote_table_name("posts.title")`) is regarded as an unsafe SQL string even though a result of `quote_table_name` should obviously be regarded as a safe SQL string, since the column name matcher doesn't respect quotation, it is a little annoying. This changes the column name matcher to allow quoted identifiers as safe SQL string, now all results of the `quote_table_name` are regarded as safe SQL string.
* Use `capture_sql` instead of `assert_sql` with no patternRyuta Kamizono2019-05-221-1/+1
| | | | And no longer need to except SCHEMA SQLs manually since 0810c07.
* Fix `count(:all)` with eager loading and explicit select and orderRyuta Kamizono2019-04-041-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This follows up ebc09ed9ad9a04338138739226a1a92c7a2707ee. We've still experienced a regression for `size` (`count(:all)`) with eager loading and explicit select and order when upgrading Rails to 5.1. In that case, the eager loading enforces `distinct` to subselect but still keep the custom select, it would cause the ORDER BY with DISTINCT issue. ``` % ARCONN=postgresql bundle exec ruby -w -Itest test/cases/relations_test.rb -n test_size_with_eager_loading_and_custom_select_and_order Using postgresql Run options: -n test_size_with_eager_loading_and_custom_select_and_order --seed 8356 # Running: E Error: RelationTest#test_size_with_eager_loading_and_custom_select_and_order: ActiveRecord::StatementInvalid: PG::InvalidColumnReference: ERROR: for SELECT DISTINCT, ORDER BY expressions must appear in select list LINE 1: ..." ON "comments"."post_id" = "posts"."id" ORDER BY comments.i... ^ ``` As another problem on `distinct` is enforced, the result of `count` becomes fewer than expected if `select` is given explicitly. e.g. ```ruby Post.select(:type).count # => 11 Post.select(:type).distinct.count # => 3 ``` As long as `distinct` is enforced, we need to care to keep the result of `count`. This fixes both the `count` with eager loading problems.
* Add `ActiveRecord::Relation#extract_associated` for extracting associated ↵David Heinemeier Hansson2019-03-291-0/+7
| | | | | record (#35784) * Add `ActiveRecord::Relation#extract_associated` for extracting associated records from a relation
* Delegate `only` query method to relation as with `except`Ryuta Kamizono2019-03-071-0/+4
| | | | | | | | | | | I've found the skewness of delegation methods between `except` and `only` in a88b6f2. The `only` method is closely similar with `except` as `SpawnMethods`. https://github.com/rails/rails/blob/e056b9bfb07c4eb3bcc6672d885aadd72bec574f/activerecord/lib/active_record/relation/spawn_methods.rb#L53-L67 It is preferable both behaves the same way.
* Relax table name detection in `from` to allow any extension like INDEX hintRyuta Kamizono2019-03-011-10/+22
| | | | | | | | | | #35360 allows table name qualified if `from` has original table name. But that is still too strict. We have a valid use case that `from` with INDEX hint (e.g. `from("comments USE INDEX (PRIMARY)")`). So I've relaxed the table name detection in `from` to allow any extension like INDEX hint. Fixes #35359.
* Fix random CI failure due to non-deterministic sorting orderRyuta Kamizono2019-02-271-5/+5
| | | | https://buildkite.com/rails/rails/builds/59106#596284a1-4692-4640-8a50-c4286e173bbb/115-126
* fixes different `count` calculation when using `size` manual `select` with ↵jvillarejo2019-02-261-0/+7
| | | | | | | | DISTINCT When using `select` with `'DISTINCT( ... )'` if you use method `size` on a non loaded relation it overrides the column selected by passing `:all` so it returns different value than count. This fixes #35214
* Add test case for `unscope` with `merge`Ryuta Kamizono2019-02-241-0/+13
|
* Add test case for `unscope` with unknown columnRyuta Kamizono2019-02-241-0/+11
|
* Make `test_select_with_subquery_in_from_uses_original_table_name` work with ↵yuuji.yaginuma2019-02-231-7/+2
| | | | | | | | | | | | | | old SQLite3 It seems that the reason why the `test_select_with_subquery_in_from_uses_original_table_name` does not pass is that the return value of `sqlite3_column_name()` is wrong due to subquery flattening. This seems to have been fixed with SQLite 3.20.0(https://sqlite.org/changes.html#version_3_20_0). But CI uses the old version(maybe 3.11.0), I added `DISTINCT` to avoid optimization by subquery flattening. Ref: https://sqlite.org/optoverview.html#flattening
* Skip `test_select_with_subquery_in_from_uses_original_table_name` on CIRyuta Kamizono2019-02-231-1/+1
| | | | Somehow `ENV["BUILDKITE"]` didn't work as expected.
* Skip `test_select_with_subquery_in_from_uses_original_table_name` on ↵Ryuta Kamizono2019-02-221-2/+5
| | | | | | Buildkite as well https://buildkite.com/rails/rails/builds/58981#2423c707-7c56-4639-a76e-8db4fd1e5cf3/102-111
* Just skip `test_select_with_subquery_in_from_uses_original_table_name` on TravisRyuta Kamizono2019-02-221-0/+3
| | | | | | | | | I'm not sure why the test is failed on Travis, it passed on locally. I suspect that failure is a bug on SQLite3, so just skip the test for now, since it was not covered by before. https://travis-ci.org/rails/rails/jobs/496726410#L1198-L1208
* Fix `pluck` and `select` with `from` if `from` has original table nameRyuta Kamizono2019-02-221-0/+36
| | | | | | | | | | | | | | | This is caused by 0ee96d1. Since #18744, `select` columns doesn't be qualified by table name if using `from`. 0ee96d1 follows that for `pluck` as well. But people depends that `pluck` columns are qualified even if using `from`. So I've fixed that to be qualified if `from` has the original table name to keep the behavior as much as before. Fixes #35359.
* Introduce delete_by and destroy_by methods to ActiveRecord::RelationAbhay Nikam2019-02-191-0/+18
|
* Merge pull request #35274 from AlexBrodianoi/fix_does_not_support_reverseRyuta Kamizono2019-02-171-0/+9
|\ | | | | Raise ActiveRecord::IrreversibleOrderError if nulls first/last is not a single ordering argument.
| * Raise ActiveRecord::IrreversibleOrderError if nulls first/last is not a ↵Finn Young2019-02-171-0/+9
| | | | | | | | single ordering argument.
* | Fix `order` with custom attributesRyuta Kamizono2019-02-171-0/+7
| | | | | | | | This follows up 0ee96d13de29680e148ccb8e5b68025f29fd091c.
* | Deprecate using class level querying methods if the receiver scope regarded ↵Ryuta Kamizono2019-02-151-12/+18
| | | | | | | | | | | | | | | | | | | | as leaked This deprecates using class level querying methods if the receiver scope regarded as leaked, since #32380 and #35186 may cause that silently leaking information when people upgrade the app. We need deprecation first before making those.
* | Revert "Merge pull request #35186 from ↵Ryuta Kamizono2019-02-151-9/+33
|/ | | | | | | | | | | | kamipo/fix_leaking_scope_on_relation_create" This reverts commit b67d5c6dedbf033515a96a95d24d085bf99a0d07, reversing changes made to 2e018361c7c51e36d1d98bf770b7456d78dee68b. Reason: #35186 may cause that silently leaking information when people upgrade the app. We need deprecation first before making this.
* Fix `pluck` and `select` with custom attributesRyuta Kamizono2019-02-131-0/+7
| | | | | | | | | Currently custom attributes are always qualified by the table name in the generated SQL wrongly even if the table doesn't have the named column, it would cause an invalid SQL error. Custom attributes should only be qualified if the table has the same named column.
* Fix `relation.create` to avoid leaking scope to initialization block and ↵Ryuta Kamizono2019-02-071-3/+18
| | | | | | | | | | | | | | | | | | | | callbacks `relation.create` populates scope attributes to new record by `scoping`, it is necessary to assign the scope attributes to the record and to find STI subclass from the scope attributes. But the effect of `scoping` is class global, it was caused undesired behavior that pollute all class level querying methods in initialization block and callbacks (`after_initialize`, `before_validation`, `before_save`, etc), which are user provided code. To avoid the leaking scope issue, restore the original current scope before initialization block and callbacks are invoked. Fixes #9894. Fixes #17577. Closes #31526.
* Relation no longer respond to Arel methodsRyuta Kamizono2019-02-061-15/+0
| | | | This follows up d97980a16d76ad190042b4d8578109714e9c53d0.
* #create_or_find_by/!: add more tests and fix docs (#34653)Bogdan2018-12-081-0/+39
| | | | | | | | * `#create_or_find_by/!`: add more tests * Fix docs of `create_or_find_by` This method uses `find_by!` internally.
* Generate delegation methods to named scope in the definition timeRyuta Kamizono2018-10-091-0/+10
| | | | | | | | | | | | | | | | | | | The delegation methods to named scope are defined when `method_missing` is invoked on the relation. Since #29301, the receiver in the named scope is changed to the relation like others (e.g. `default_scope`, etc) for consistency. Most named scopes would be delegated from relation by `method_missing`, since we don't allow scopes to be defined which conflict with instance methods on `Relation` (#31179). But if a named scope is defined with the same name as any method on the `superclass` (e.g. `Kernel.open`), the `method_missing` on the relation is not invoked. To address the issue, make the delegation methods to named scope is generated in the definition time. Fixes #34098.
* Extract `{update,delete}_all_test.rb` from `persistence_test.rb` and ↵Ryuta Kamizono2018-09-161-180/+0
| | | | | | | | `relations_test.rb` `persistence_test.rb` and `relations_test.rb` have too many lines, so I'd like to extract relation around tests to dedicated files before newly test added.
* Fixes #33610Darwin D Wu2018-09-111-0/+11
| | | | | | | | | | | | In order to avoid double assignments of nested_attributes for `has_many` relations during record initialization, nested_attributes in `create_with` should not be passed into `klass.new` and have them populate during `initialize_internals_callback` with scope attributes. However, `create_with` keys should always have precedence over where clauses, so if there are same keys in both `create_with` and `where_values_hash`, the value in `create_with` should be the one that's used.
* Just delegate `update` with ids on a relation to `klass.update`Ryuta Kamizono2018-08-311-0/+18
| | | | | | | | | | | | This restores an ability that `update` with ids on a relation which is described at https://github.com/rails/rails/issues/33470#issuecomment-411203013. I personally think that the `update` with two arguments on a relation is not a designed feature, since that is totally not using a relation state, and also is not documented. But removing any feature should not be suddenly happened in a stable version even if that is not documented.
* Avoid extra scoping when using `Relation#update`Ryuta Kamizono2018-07-311-0/+4
| | | | | | | | | | | | | | Since 9ac7dd4, class level `update`, `destroy`, and `delete` were placed in the `Persistence` module as class methods. But `Relation#update` without passing ids which was introduced at #11898 is not a class method, and it was caused the extra scoping regression #33470. I moved the relation method back into the `Relation` to fix the regression. Fixes #33470.
* Merge pull request #31513 from fatkodima/relation-touch_allRafael França2018-04-201-1/+46
|\ | | | | Add `touch_all` method to `ActiveRecord::Relation`
| * Add `touch_all` method to `ActiveRecord::Relation`fatkodima2018-04-131-1/+46
| |
* | Replace `assert !` with `assert_not`Daniel Colson2018-04-191-8/+8
|/ | | | | This autocorrects the violations after adding a custom cop in 3305c78dcd.
* Merge pull request #32005 from maschwenk/ar-distinct-order-count-regressionRyuta Kamizono2018-02-271-0/+6
| | | | Active Record distinct & order #count regression
* Add #create_or_find_by to lean on unique constraints (#31989)David Heinemeier Hansson2018-02-141-0/+29
| | | Add #create_or_find_by to lean on unique constraints
* Remove extra whitespaceDaniel Colson2018-01-251-14/+14
|
* Use assert_empty and assert_not_emptyDaniel Colson2018-01-251-4/+4
|
* Use assert_predicate and assert_not_predicateDaniel Colson2018-01-251-62/+62
|
* Use respond_to test helpersDaniel Colson2018-01-251-5/+5
|
* Fix `count(:all)` with eager loading and having an order other than the ↵Ryuta Kamizono2018-01-251-0/+6
| | | | | | | | | | | | | | | | | | | driving table This is a regression caused by 6beb4de. In PostgreSQL, ORDER BY expressions must appear in SELECT list when using DISTINCT. When using `count(:all)` with eager loading, Active Record enforces DISTINCT to count the driving table records only. 6beb4de was caused the regression because `count(:all)` with DISTINCT path no longer removes ORDER BY. We need to ignore ORDER BY when DISTINCT is enforced, otherwise not always generated valid SQL for PostgreSQL. Fixes #31783.
* Avoid passing unnecessary arguments to relationDaniel Colson2018-01-241-1/+5
| | | | | | | | | | | | Most of the time the table and predicate_builder passed to Relation.new are exactly the arel_table and predicate builder of the given klass. This uses klass.arel_table and klass.predicate_builder as the defaults, so we don't have to pass them in most cases. This does change the signaure of both Relation and AssocationRelation. Are we ok with that?
* Fix `count(:all)` to correctly work `distinct` with custom SELECT listRyuta Kamizono2017-12-201-0/+6
| | | | | | | | | | | | | | Currently `count(:all)` with `distinct` doesn't work correctly because SELECT list is always replaced to `*` or primary key in that case even if having custom SELECT list. And also, PostgreSQL has a limitation that ORDER BY expressions must appear in select list for SELECT DISTINCT. Therefore, we should not replace custom SELECT list when using `count(:all)` with `distinct`. Closes #31277.
* Remove outdated comments [ci skip]Ryuta Kamizono2017-12-191-4/+0
|
* Using subselect for `delete_all` with `limit` or `offset`Ryuta Kamizono2017-12-191-2/+0
| | | | | | Arel doesn't support subselect generation for DELETE unlike UPDATE yet, but we already have that generation in connection adapters. We can simply use the subselect generated by that one.
* Fix `scope_for_create` to do not lose polymorphic associationsRyuta Kamizono2017-12-081-0/+9
| | | | | | | | | | | | This regression was caused at 213796fb due to polymorphic predicates are combined by `Arel::Nodes::And`. But I'd like to keep that combined because it would help inverting polymorphic predicates correctly (e9ba12f7), and we can collect equality nodes regardless of combined by `Arel::Nodes::And` (`a AND (b AND c) AND d` == `a AND b AND c AND d`). This change fixes the regression to collect equality nodes in `Arel::Nodes::And` as well. Fixes #31338.
generated by cgit v1.2.3 (git 2.25.1) at 2024-06-18 13:47:40 +0000