aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/lib/active_record/relation/finder_methods.rb
Commit message (Collapse)AuthorAgeFilesLines
* Merge pull request #36647 from ↵Ryuta Kamizono2019-07-111-1/+1
|\ | | | | | | | | | | giraffate/fix_exists_with_distinct_and_offset_and_order_in_postgresql Fix `relation.exists?` with giving `distinct`, `offset` and `order` for joined table
| * Fix `relation.exists?` with giving `distinct`, `offset` and `order` for ↵Takayuki Nakata2019-07-101-1/+1
|/ | | | | | | | | | | joined table The error happens in PostgreSQL when using `relation.exists?` with `distinct`, `offset` and `order` for joined table. However, the error does not happen if either `distinct` or `offset` is removed. This behavior is confusing. Fixes #36632
* Enable `Layout/EmptyLinesAroundAccessModifier` copRyuta Kamizono2019-06-131-1/+0
| | | | | | | | | | | 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.
* Fix merging left_joins to maintain its own `join_type` contextRyuta Kamizono2019-04-271-1/+3
| | | | | | | | | | | | | | | This fixes a regression for #35864. Usually, stashed joins (mainly eager loading) are performed as LEFT JOINs. But the case of merging joins/left_joins of different class, that (stashed) joins are performed as the same `join_type` as the parent context for now. Since #35864, both (joins/left_joins) stashed joins might be contained in `joins_values`, so each stashed joins should maintain its own `join_type` context. Fixes #36103.
* Clarify exists check in logsDan Fitch2019-04-091-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The default log messages for Model.exists?, when called from .save on an object which uses scoped uniqueness validation like: class Example < ApplicationRecord validates :field, uniqueness: {scope: parent_id} end can result in slightly misleading logs. An example case: ↳ app/controllers/example_controller.rb:23 (0.2ms) begin transaction ↳ app/controllers/example_controller.rb:39 Example Exists (0.2ms) SELECT 1 AS one FROM "examples" WHERE "examples"."field" IS NULL AND "examples"."parent_id" = ? LIMIT ? [["parent_id", 123], ["LIMIT", 1]] ↳ app/controllers/example_controller.rb:39 (0.1ms) rollback transaction To me, a Rails newbie, this parsed as the following: - started the transaction to create a thing - found that your object exists already! - so we rolled back the transaction (even though the actual cause of the transaction is something that happens after the Exists check.) All this does is add a question mark to the message, to make it clear in the log that this is a check, not a confirmation. This may be kind of silly, but it may save some future goofs by newbs like me.
* Stash `left_joins` into `joins` to deduplicate redundant LEFT JOINRyuta Kamizono2019-04-051-6/+0
| | | | | | | | | | | | | | | | | | | | | | | | | Originally the `JoinDependency` has the deduplication for eager loading (LEFT JOIN). This re-uses that deduplication for `left_joins`. And also, This makes left join order into part of joins, i.e.: Before: ``` association joins -> stash joins (eager loading, etc) -> string joins -> left joins ``` After: ``` association joins -> stash joins (eager loading, left joins, etc) -> string joins ``` Now string joins are able to refer left joins. Fixes #34325. Fixes #34332. Fixes #34536.
* Replace “can not” with “cannot”.Samantha John2019-03-061-1/+1
|
* [ci skip] The `find` method coerces the given arguments to integer if the ↵Mehmet Emin INAC2019-02-281-1/+1
| | | | `primary key` is integer
* [ci skip] Fix the documentation of ActiveRecord::FinderMethods#findMehmet Emin INAC2019-02-281-2/+2
| | | | | | | It's mentioned everywhere as `ActiveRecord::RecordNotFound` so to be coherent with the rest of the documentation I've applied it here. Also doc was saying if the parameter is integer it coerces it which is other way around.
* Fix `relation.exists?` with giving both `distinct` and `offset`Ryuta Kamizono2019-02-081-4/+8
| | | | | | | | | | | The `distinct` affects (reduces) rows of the result, so it is important part when both `distinct` and `offset` are given. Replacing SELECT clause to `1 AS one` and removing `distinct` and `order` is just optimization for the `exists?`, we should not apply the optimization for that case. Fixes #35191.
* All of queries should return correct result even if including large numberRyuta Kamizono2019-01-181-10/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently several queries cannot return correct result due to incorrect `RangeError` handling. First example: ```ruby assert_equal true, Topic.where(id: [1, 9223372036854775808]).exists? assert_equal true, Topic.where.not(id: 9223372036854775808).exists? ``` The first example is obviously to be true, but currently it returns false. Second example: ```ruby assert_equal topics(:first), Topic.where(id: 1..9223372036854775808).find(1) ``` The second example also should return the object, but currently it raises `RecordNotFound`. It can be seen from the examples, the queries including large number assuming empty result is not always correct. Therefore, This change handles `RangeError` to generate executable SQL instead of raising `RangeError` to users to always return correct result. By this change, it is no longer raised `RangeError` to users.
* Allow strong params in ActiveRecord::Base#exists?Gannon McGibbon2019-01-071-0/+2
| | | | | Allow `ActionController::Params` as argument of `ActiveRecord::Base#exists?`
* Make implicit order column configurableTekin Suleyman2018-11-261-2/+2
| | | | | | | | | | | | | | When calling ordered finder methods such as +first+ or +last+ without an explicit order clause, ActiveRecord sorts records by primary key. This can result in unpredictable and surprising behaviour when the primary key is not an auto-incrementing integer, for example when it's a UUID. This change makes it possible to override the column used for implicit ordering such that +first+ and +last+ will return more predictable results. For Example: class Project < ActiveRecord::Base self.implicit_order_column = "created_at" end
* Ignore empty condition on #construct_relation_for_existsr7kamura2018-10-271-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | At https://github.com/rails/rails/commit/fc0e3354af7e7878bdd905a95ce4c1491113af9a, ```rb relation = relation.where(conditions) ``` was rewritten to: ```rb relation.where!(condition) ``` This change accidentally changed the result of `Topic.exists?({})` from true to false. To fix this regression, first I moved the blank check logic (`opts.blank?`) from `#where` to `#where!`, because I thought `#where!` should be identical to `#where`, except that instead of returning a new relation, it adds the condition to the existing relation. But on second thought after some discussion on https://github.com/rails/rails/pull/34329, I started to think that just fixing `#construct_relation_for_exists` is more preferable than changing `#where` and `#where!`.
* Enable `Performance/UnfreezeString` copyuuji.yaginuma2018-09-231-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In Ruby 2.3 or later, `String#+@` is available and `+@` is faster than `dup`. ```ruby # frozen_string_literal: true require "bundler/inline" gemfile(true) do source "https://rubygems.org" gem "benchmark-ips" end Benchmark.ips do |x| x.report('+@') { +"" } x.report('dup') { "".dup } x.compare! end ``` ``` $ ruby -v benchmark.rb ruby 2.5.1p57 (2018-03-29 revision 63029) [x86_64-linux] Warming up -------------------------------------- +@ 282.289k i/100ms dup 187.638k i/100ms Calculating ------------------------------------- +@ 6.775M (± 3.6%) i/s - 33.875M in 5.006253s dup 3.320M (± 2.2%) i/s - 16.700M in 5.032125s Comparison: +@: 6775299.3 i/s dup: 3320400.7 i/s - 2.04x slower ```
* Don't return the same object when using find with an empty arrayRafael Mendonça França2018-09-191-1/+1
| | | | | | When you pass an empty array to find we know we shoudl return an empty array but it is surprising that we are returning the original empty array instead of a new one.
* Use `visitor.compile` instead of constructing by connection itselfRyuta Kamizono2018-09-091-1/+1
|
* Revert "Merge pull request #24131 from brchristian/limit_and_primary_key"Ryuta Kamizono2018-08-011-1/+1
| | | | | | | | | | This reverts commit d162188dd662a7d9f62ba8431474f50bc35e3e93, reversing changes made to 3576782888c307e3e192c44e332b957cd1174128. Reason: #24131 conflicts the #5153's default order contract, it means that existing apps would be broken by that change. We don't want to break existing apps without a deprecation cycle.
* don't impose primary key order if limit() is definedBrian Christian2018-07-191-1/+1
|
* Use `construct_join_dependency` in all placesRyuta Kamizono2018-07-031-4/+3
|
* Ensure to calculate column aliases after all table aliases are constructedRyuta Kamizono2018-06-191-3/+1
| | | | | | | | | | | | | | | | | Currently, column aliases which is used for eager loading are calculated before constructing all table aliases in FROM clause. `JoinDependency#join_constraints` constructs table aliases for `joins` first, and then always re-constructs table aliases for eager loading. If both `joins` and eager loading are given a same table association, the re-construction would cause the discrepancy between column aliases and table aliases. To avoid the discrepancy, the column aliases should be calculated after all table aliases are constructed. Fixes #30603.
* Fix GROUP BY queries to apply LIMIT/OFFSET after aggregationsRyuta Kamizono2018-06-071-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If `eager_loading` is true, `apply_join_dependency` force applies LIMIT/OFFSET before JOINs by `limited_ids_for` to keep parent records count. But for aggregation queries, LIMIT/OFFSET should be applied after aggregations the same as SQL semantics. And also, we could not replace SELECT list by `limited_ids_for` when a query has a GROUP BY clause. It had never been worked since it will causes generating invalid SQL for MySQL, PostgreSQL, and probably most backends. ``` % ARCONN=postgresql be ruby -w -Itest test/cases/calculations_test.rb -n test_group_by_with_limit Using postgresql Run options: -n test_group_by_with_limit --seed 20925 # Running: E Error: CalculationsTest#test_group_by_with_limit: ActiveRecord::StatementInvalid: PG::GroupingError: ERROR: column "posts.id" must appear in the GROUP BY clause or be used in an aggregate function LINE 1: SELECT DISTINCT "posts"."id", "posts"."type" AS alias_0 FRO... ^ : SELECT DISTINCT "posts"."id", "posts"."type" AS alias_0 FROM "posts" LEFT OUTER JOIN "comments" ON "comments"."post_id" = "posts"."id" GROUP BY "posts"."type" ORDER BY "posts"."type" ASC LIMIT $1 ``` Fixes #8103. Closes #27249.
* Eager loading with polymorphic associations should behave consistentlyRyuta Kamizono2018-03-041-3/+3
| | | | | | | | | | This reverts ignoring polymorphic error introduced at 02da8ae. What the ignoring want to solve was caused by force eager loading regardless of whether it is necessary, but it has been fixed by #29043. The ignoring is now only causing a mismatch of `exists?` behavior with `to_a`, `count`, etc. It should behave consistently.
* Don't pass garbage args to alias trackerRyuta Kamizono2018-01-141-1/+2
| | | | | | | | | | | | | This is a complete fix to #30995. Originally alias tracker will only track table aliases on `Arel::Nodes::Join`, other args are ignored. Since c5ab6e5, parent aliases hash will be passed then it caused the regression #30995. It is enough to pass list of `Arel::Nodes::Join` simply, not need to pass garbage args which will be ignored.
* Use `apply_join_dependency` instead of meaningless named ↵Ryuta Kamizono2018-01-111-24/+11
| | | | | | | | | | `find_with_associations` `find_with_associations` is meaningless name in this point since it just contain `construct_join_dependency` and `apply_join_dependency`, does not contain finding anything. If `apply_join_dependency` returns `relation` and `join_dependency` then `find_with_associations` is no longer needed.
* Make `relation.exists?` more performant when using eager loadingRyuta Kamizono2018-01-111-3/+4
| | | | | | `relation.exists?` just wants to know if there is a result or not, does not need the exact records matched. Therefore, an intermediate SELECT query for eager loading is not necessary.
* resolve inconsistencies between first and to_a.first with limitBrian Christian2018-01-091-1/+5
|
* Make `find_nth_from_last` more performant when using reversible orderRyuta Kamizono2018-01-071-6/+5
| | | | | We can use `relation.last(index)[-index]` instead of loading all records when using reversible order because `last` will call `reverse_order`.
* Fix `last` with `offset` to behave consistently with loaded relationRyuta Kamizono2018-01-071-1/+1
| | | | | | Currently `last` with `offset` behaves incorrectly because `offset` can not be reversed like `limit`. Therefore, `offset` should also be handled like `limit`.
* Fix `pluck` with eager loading to respect `offset`Ryuta Kamizono2018-01-071-1/+1
|
* Update docs `ActiveRecord::FinderMethods#find`suginoy2017-11-281-3/+4
| | | | ref https://github.com/rails/rails/pull/22653
* Provide arguments to RecordNotFoundNikita Misharin2017-11-251-5/+9
|
* Update `exists?` documentationNikolai B2017-11-141-1/+2
| | | | Make it clear that `exists?` can be chained onto a relation
* Ensure `apply_join_dependency` for `collection_cache_key` if eager-loading ↵Ryuta Kamizono2017-11-061-1/+1
| | | | | | is needed Fixes #30315.
* Remove meaningless named `construct_relation_for_association_calculations`Ryuta Kamizono2017-10-091-4/+0
| | | | | I don't think this is a good abstraction because the internal method is used only if the relation need to be applied join dependency.
* Fix `relation.exists?` with has_many through associationsRyuta Kamizono2017-10-091-4/+4
| | | | | `relation.exists?` should reference correct aliases while joining tables of has_many through associations.
* Remove passing redundant `self` to internal `apply_join_dependency` etcRyuta Kamizono2017-10-091-13/+12
|
* Decouple building `AliasTracker` from `JoinDependency`Ryuta Kamizono2017-10-081-1/+3
| | | | | This is preparation to respect parent relation's alias tracking for fixing #30681.
* Don't use `quoted_table_name` in `limited_ids_for`Ryuta Kamizono2017-09-141-1/+3
| | | | | | Because `quoted_table_name` doesn't respect table alias. We should use `arel_attribute` for that, so I added `column_name_from_arel_node` to generate column name from an arel node.
* Should work inverse association when eager loadingRyuta Kamizono2017-08-251-10/+1
| | | | | | | This regression was caused by caa178c1. The block for `set_inverse_instance` should also be passed to join dependency. Fixes #30402.
* Return Not found Ids in ActiveRecord::NotFoundGaurish Sharma2017-07-291-3/+3
| | | | | This builds on top of 15e2da656f41af0124f7577858536f3b65462ad5. now it also returns exact Ids which were not found which will be debugging simple.
* Refactor Active Record to let Arel manage bind paramsSean Griffin2017-07-241-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A common source of bugs and code bloat within Active Record has been the need for us to maintain the list of bind values separately from the AST they're associated with. This makes any sort of AST manipulation incredibly difficult, as any time we want to potentially insert or remove an AST node, we need to traverse the entire tree to find where the associated bind parameters are. With this change, the bind parameters now live on the AST directly. Active Record does not need to know or care about them until the final AST traversal for SQL construction. Rather than returning just the SQL, the Arel collector will now return both the SQL and the bind parameters. At this point the connection adapter will have all the values that it had before. A bit of this code is janky and something I'd like to refactor later. In particular, I don't like how we're handling associations in the predicate builder, the special casing of `StatementCache::Substitute` in `QueryAttribute`, or generally how we're handling bind value replacement in the statement cache when prepared statements are disabled. This also mostly reverts #26378, as it moved all the code into a location that I wanted to delete. /cc @metaskills @yahonda, this change will affect the adapters Fixes #29766. Fixes #29804. Fixes #26541. Close #28539. Close #24769. Close #26468. Close #26202. There are probably other issues/PRs that can be closed because of this commit, but that's all I could find on the first few pages.
* Use frozen-string-literal in ActiveRecordKir Shatrov2017-07-191-0/+2
|
* Fix `JoinDependency` with using a custom tableRyuta Kamizono2017-07-181-1/+1
| | | | | | | | | | | | | | | Without this fix, `JoinDependency` doesn't use a custom table alias: ``` % ARCONN=sqlite3 be ruby -w -Itest test/cases/relations_test.rb -n test_using_a_custom_table_with_joins_affects_the_wheres Using sqlite3 Run options: -n test_using_a_custom_table_with_joins_affects_the_wheres --seed 14531 E Error:RelationTest#test_using_a_custom_table_with_joins_affects_the_wheres: ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: posts.author_id: SELECT "omg_posts".* FROM "posts" "omg_posts" INNER JOIN "authors" ON "authors"."id" = "posts"."author_id" WHERE "omg_posts"."title" = ? LIMIT ? ```
* Remove useless `arel_engine`Ryuta Kamizono2017-07-171-1/+1
| | | | | | | | | | | `arel_engine` is only used in `raise_record_not_found_exception!` to use `engine.connection` (and `connection.visitor`) in `arel.where_sql`. https://github.com/rails/arel/blob/v8.0.0/lib/arel/select_manager.rb#L183 But `klass.connection` will work as expected even if not using `arel_engine` (described by `test_connection`). So `arel_engine` is no longer needed.
* Skip query cache for in_batches and friendsEugene Kenny2017-07-061-3/+3
| | | | | | | | | | | The `find_each`, `find_in_batches` and `in_batches` APIs usually operate on large numbers of records, where it's preferable not to load them all into memory at once. If the query cache is enabled, it will hold onto the query results until the end of the execution context (request/job), which means the memory used is still proportional to the total number of records. These queries are typically not repeated, so the query cache isn't desirable here.
* Revert "Merge pull request #29540 from kirs/rubocop-frozen-string"Matthew Draper2017-07-021-1/+0
| | | | | This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
* Merge pull request #29540 from kirs/rubocop-frozen-stringMatthew Draper2017-07-021-0/+1
|\ | | | | | | Enforce frozen string in Rubocop
| * Enforce frozen string in RubocopKir Shatrov2017-07-011-0/+1
| |
* | Merge pull request #29506 from pat/frozen-string-literalsMatthew Draper2017-07-021-2/+2
|\ \ | |/ |/| | | Make ActiveSupport frozen-string-literal friendly.
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-05 21:10:03 +0000