aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/lib/active_record/relation/finder_methods.rb
Commit message (Collapse)AuthorAgeFilesLines
* 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.
| * Make ActiveRecord frozen string literal friendly.Pat Allan2017-06-201-2/+2
| |
* | Merge pull request #29405 from kamipo/locked_should_not_build_arelRafael França2017-06-281-5/+3
|\ \ | | | | | | `Relation#locked?` should not build arel
| * | Remove delegating to arel in a relationRyuta Kamizono2017-06-291-5/+3
| |/ | | | | | | | | The delegation was needed since passing `relation` with `relation.bound_attributes`. It should use `relation.arel` in that case.
* / Extract `ordered_relation` in `FinderMethods`Ryuta Kamizono2017-06-241-12/+11
|/
* Don't eager loading if unneeded for `FinderMethods#exists?`Ryuta Kamizono2017-05-111-2/+4
| | | | Fixes #29025.
* exclude ORDER BY clause for exists? (#28699)Boris Slobodin2017-04-101-1/+1
|
* Extract `construct_relation_for_exists` in `FinderMethods`Ryuta Kamizono2017-04-091-12/+15
| | | | To ease to customize a relation for `exists?`.
* Revert "Merge pull request #28598 from wnadeau/patch-1"Rafael Mendonça França2017-04-031-1/+1
| | | | | | | | This reverts commit a680a5814184e2f37c4686aa53d0ad3c7fb6b1ee, reversing changes made to 842f67dd242e738419f27e752ea7dcd0bbe87b6d. Reason: I can't resist to the joke, so better to keep it there https://github.com/rails/rails/pull/28598#issuecomment-290945339.
* FinderMethods#fourty_two docs cite proper sourceWinfred Nadeau2017-03-291-1/+1
| | | | | | | silly method gets a silly doc fix, or I'm missing an even sillier joke and I'm about to get schooled. BUT I'm pretty sure this is some serious Beaudrillard simulacrum, though. I'm just doing my part to spread the gospel of Douglas Adams.
* Fix `find_nth` with `limit_value`Ryuta Kamizono2017-02-261-3/+7
| | | | | If the `index` exceeds a `limit`, simply return an empty result without querying the database.
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-04 21:10:10 +0000