aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/test/cases/relation_test.rb
Commit message (Collapse)AuthorAgeFilesLines
* Properly cast input in `update_all`Sean Griffin2017-11-131-1/+6
| | | | | | | | | | | | | | The documentation claims that given values go through "normal AR type casting and serialization", which to me implies `serialize(cast(value))`, not just serialization. The docs were changed to use this wording in #22492. The tests I cited in that PR (which is the same test modified in this commit), is worded in a way that implies it should be using `cast` as well. It's possible that I originally meant "normal type casting" to imply just the call to `serialize`, but given that `update_all(archived: params['archived'])` seems to be pretty common, I'm inclined to make this change as long as no tests are broken from it.
* Relation merging should keep joining orderRyuta Kamizono2017-11-111-0/+9
| | | | | | | `joins_values.partition` will break joins values order. It should be kept as user intended order. Fixes #15488.
* Remove unused explicit delegation to `klass` in `relation`Ryuta Kamizono2017-09-141-5/+0
| | | | | | | It is only used `primary_key` and `connection` in the internal, so it is not needed to delegate others to `klass` explicitly. This doesn't change public behavior because `relation` will delegate missing method to `klass`.
* `Relation::Merger` should not fill `values` with empty valuesRyuta Kamizono2017-07-251-0/+8
| | | | | | | | | Currently `Relation#merge` will almost fill `values` with empty values (e.g. `other.order_values` is always true, it should be `other.order_values.any?`). This means that `Relation#merge` always changes `values` even if actually `values` is nothing changed. This behavior will makes `Relation#empty_scope?` fragile. So `Relation#merge` should avoid unnecessary changes.
* Refactor Active Record to let Arel manage bind paramsSean Griffin2017-07-241-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
|
* Don't cache `scope_for_create`Ryuta Kamizono2017-07-161-10/+2
| | | | | | | | | | | | | | | | | | | | | | | | | I investigated where `scope_for_create` is reused in tests with the following code: ```diff --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -590,6 +590,10 @@ def where_values_hash(relation_table_name = table_name) end def scope_for_create + if defined?(@scope_for_create) && @scope_for_create + puts caller + puts "defined" + end @scope_for_create ||= where_values_hash.merge!(create_with_value.stringify_keys) end ``` It was hit only `test_scope_for_create_is_cached`. This means that `scope_for_create` will not be reused in normal use cases. So we can remove caching `scope_for_create` to respect changing `where_clause` and `create_with_value`.
* Fix `create_with` using both string and symbolRyuta Kamizono2017-07-161-4/+3
| | | | | | | This is related with #27680. Since `where_values_hash` keys constructed by `where` are string, so we need `stringify_keys` to `create_with_value` before merging it.
* Use `where(id: 10)` rather than `where(relation.table[:id].eq(10))`Ryuta Kamizono2017-07-161-5/+5
| | | | | Because Arel is a private API and to describe `where_values_hash` keys constructed by `where` are string.
* Extract `FakeKlass` in `relation_test.rb` and `relation/mutation_test.rb`Ryuta Kamizono2017-07-111-19/+1
| | | | | `FakeKlass` in `relation_test.rb` and `relation/mutation_test.rb` are almost the same.
* 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.
* Enforce frozen string in RubocopKir Shatrov2017-07-011-0/+1
|
* Remove unused variableyuuji.yaginuma2017-06-221-1/+1
| | | | | | | | This fixes the following warnings: ``` activerecord/test/cases/relation_test.rb:231: warning: assigned but unused variable - authors_with_commented_posts ```
* Keep INNER JOIN when merging relationsMaxime Lapointe2017-06-201-2/+21
| | | | | | | | | | | | | | | | | Doing `Author.joins(:posts).merge(Post.joins(:comments))` does this `SELECT ... INNER JOIN posts ON... LEFT OUTER JOIN comments ON...` instead of doing `SELECT ... INNER JOIN posts ON... INNER JOIN comments ON...`. This behavior is unexpected and makes little sense as, basically, doing `Post.joins(:comments)` means I want posts that have comments. Turning it to a LEFT JOIN means I want posts and join the comments data, if any. We can see this problem directly in the existing tests. The test_relation_merging_with_merged_joins_as_symbols only does joins from posts to comments to ratings while the ratings fixture isn't loaded, but the count is non-zero.
* Restore `fixtures :author_addresses`Ryuta Kamizono2017-04-271-1/+1
| | | | | This change reverted in eac6f369 but it is needed for data integrity. See #25328.
* Revert "Merge pull request #27636 from ↵Rafael Mendonça França2017-04-261-1/+1
| | | | | | | | | mtsmfm/disable-referential-integrity-without-superuser-privilege-take-2" This reverts commit c1faca6333abe4b938b98fedc8d1f47b88209ecf, reversing changes made to 8c658a0ecc7f2b5fc015d424baf9edf6f3eb2b0b. See https://github.com/rails/rails/pull/27636#issuecomment-297534129
* Use `SET CONSTRAINTS` for `disable_referential_integrity` without superuser ↵Fumiaki MATSUSHIMA2017-03-261-1/+1
| | | | | | | | | | | | | | | privileges (take 2) Re-create https://github.com/rails/rails/pull/21233 eeac6151a5 was reverted (127509c071b4) because it breaks tests. ---------------- ref: 72c1557254 - We must use `authors` fixture with `author_addresses` because of its foreign key constraint. - Tests require PostgreSQL >= 9.4.2 because it had a bug about `ALTER CONSTRAINTS` and fixed in 9.4.2.
* class Foo < Struct.new(:x) creates an extra unneeded anonymous classAkira Matsuda2017-01-131-1/+1
| | | | because Struct.new returns a Class, we just can give it a name and use it directly without inheriting from it
* Revert "Merge pull request #21233 from ↵Rafael Mendonça França2017-01-031-1/+1
| | | | | | | | | | mtsmfm/disable-referential-integrity-without-superuser-privileges" This reverts commit eeac6151a55cb7d5f799e1ae33aa64a839cbc3aa, reversing changes made to 5c40239d3104543e70508360d27584a3e4dc5baf. Reason: Broke the isolated tests. https://travis-ci.org/rails/rails/builds/188721346
* Use `SET CONSTRAINTS` for `disable_referential_integrity` without superuser ↵Fumiaki MATSUSHIMA2016-12-031-1/+1
| | | | | | | | | privileges ref: 72c1557254 - We must use `authors` fixture with `author_addresses` because of its foreign key constraint. - Tests require PostgreSQL >= 9.4.2 because it had a bug about `ALTER CONSTRAINTS` and fixed in 9.4.2.
* Add more rubocop rules about whitespacesRafael Mendonça França2016-10-291-3/+3
|
* Fix broken heredoc indentation caused by rubocop auto-correctRyuta Kamizono2016-09-031-4/+4
| | | | | | All indentation was normalized by rubocop auto-correct at 80e66cc4d90bf8c15d1a5f6e3152e90147f00772. But heredocs was still kept absolute position. This commit aligns heredocs indentation for consistency.
* Merge pull request #26182 from bogdan/remove-relation-metaprogrammingRafael França2016-08-231-1/+2
|\ | | | | Remove over meta programming in AR::Relation
| * Remove over meta programming in AR::RelationBogdan Gusiev2016-08-231-1/+2
| | | | | | | | | | | | | | | | | | | | Introduced low level methods #set_value and #get_value for setting query attributes: relation.set_value(:where, {id: 1}) relation.get_value(:includes) Used those internally when working with relation's attributes at the abstract level
* | Fix count which would sometimes force a DISTINCTMaxime Lapointe2016-08-161-2/+2
|/ | | | | | | | | | | | | | | | | | | | | | | | The current behaviour of checking if there is a LEFT OUTER JOIN arel node to detect if we are doing eager_loading is wrong. This problem wasn't frequent before as only some pretty specific cases would add a LEFT OUTER JOIN arel node. However, the recent new feature left_outer_joins also add this node and made this problem happen frequently. Since in the perform_calculation function, we don't have access to eager_loading information, I had to extract the logic for the distinct out to the calculate method. As I was in the file for left_outer_join tests, I fixed a few that had bugs and I replaced some that were really weak with something that will catch more issues. In relation tests, the first test I changed would have failed if it had validated the hash returned by count instead of just checking how many pairs were in it. This is because this merge of join currently transforms the join node into an outer join node, which then made count do a distinct. So before this change, the return was {1=>1, 4=>1, 5=>1}.
* Add three new rubocop rulesRafael Mendonça França2016-08-161-6/+6
| | | | | | | | Style/SpaceBeforeBlockBraces Style/SpaceInsideBlockBraces Style/SpaceInsideHashLiteralBraces Fix all violations in the repository.
* normalizes indentation and whitespace across the projectXavier Noria2016-08-061-11/+11
|
* modernizes hash syntax in activerecordXavier Noria2016-08-061-5/+5
|
* applies new string literal convention in activerecord/testXavier Noria2016-08-061-29/+29
| | | | | The current code base is not uniform. After some discussion, we have chosen to go with double quotes by default.
* activerecord: reuse immutable objectsTamir Duberstein2016-01-041-2/+6
|
* Remove FIXME comments about the `Arel::Nodes::Quoted` [ci skip]Ryuta Kamizono2016-01-031-3/+0
| | | | | The `Arel::Nodes::Quoted` was removed already. Follow up to f916aa247bddba0c58c50822886bc29e8556df76.
* Raise ArgumentError when passing a truthy value to mergeAndrew White2015-11-171-4/+7
| | | | | | In b71e08f we started raising when nil or false was passed to merge to fix #12264, however we should also do this for truthy values that are invalid like true.
* Make `AR::SpawnMethods#merge!` to check an arg is a Procyui-knk2015-11-121-0/+4
| | | | | | | | | | From Ruby ( 2.3.0dev trunk 52520), `Hash#to_proc` is defined (https://github.com/ruby/ruby/commit/fbe967ec02cb65a7efa3fb8f3d747cf6f620dde1), and many tests have been failed with `ArgumentError: wrong number of arguments (given 0, expected 1)`. Because we call `Hash#to_proc` with no args in `#merge!`. This commit changes order of conditionals to not call `Hash#to_proc`.
* Define `sanitize_sql_for_order` for AR and use it inside `preprocess_order_args`yui-knk2015-11-021-0/+4
| | | | This commit follows up of 6a6dbb4c51fb0c58ba1a810eaa552774167b758a.
* Green version of moving the handling of supported arguments to `where`yui-knk2015-10-181-7/+4
| | | | | | | | This commit follow up of 4d8f62d. The difference from 4d8f62d are below: * Change `WhereClauseFactory` to accept `Arel::Nodes::Node` * Change test cases of `relation_test.rb`
* Skip the test added in 9cc324a on buggy versions of SQliteSean Griffin2015-09-211-7/+22
| | | | See 7dcfc25e7c52682a4343c2ba7188a69e7c06c936 for more details
* Skip test that fails from outdated sqlite3 on travisSean Griffin2015-06-021-7/+9
|
* Ensure symbols passed to `select` are always quotedSean Griffin2015-05-301-0/+22
| | | | | | | | | | | | | | | | | Our general contract in Active Record is that strings are assumed to be SQL literals, and symbols are assumed to reference a column. If a from clause is given, we shouldn't include the table name, but we should still quote the value as if it were a column. Upon fixing this, the tests were still failing on SQLite. This was because the column name being returned by the query was `"\"join\""` instead of `"join"`. This is actually a bug in SQLite that was fixed a long time ago, but I was using the version of SQLite included by OS X which has this bug. Since I'm guessing this will be a common case for contributors, I also added an explicit check with a more helpful error message. Fixes #20360
* `type_cast_for_database` -> `serialize`Sean Griffin2015-02-171-1/+1
|
* `Type#type_cast_from_database` -> `Type#deserialize`Sean Griffin2015-02-171-1/+1
|
* Raise ArgumentError when passing nil to Relation#mergeRafael Mendonça França2015-02-061-0/+16
| | | | | | nil or false should not be valid argument to the merge method. Closes #12264
* `WhereClause#predicates` does not need to be publicSean Griffin2015-01-271-2/+2
| | | | | | | | | | | The only place it was accessed was in tests. Many of them have another way that they can test their behavior, that doesn't involve reaching into internals as far as they did. `AssociationScopeTest` is testing a situation where the where clause would have one bind param per predicate, so it can just ignore the predicates entirely. The where chain test was primarly duplicating the logic tested on `WhereClause` directly, so I instead just make sure it calls the appropriate method which is fully tested in isolation.
* Remove all references to `where_values` in testsSean Griffin2015-01-251-4/+4
|
* Introduce `Relation::WhereClause`Sean Griffin2015-01-251-8/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The way that bind values are currently stored on Relation is a mess. They can come from `having`, `where`, or `join`. I'm almost certain that `having` is actually broken, and calling `where` followed by `having` followed by `where` will completely scramble the binds. Joins don't actually add the bind parameters to the relation itself, but instead add it onto an accessor on the arel AST which is undocumented, and unused in Arel itself. This means that the bind values must always be accessed as `relation.arel.bind_values + relation.bind_values`. Anything that doesn't is likely broken (and tons of bugs have come up for exactly that reason) The result is that everything dealing with `Relation` instances has to know far too much about the internals. The binds are split, combined, and re-stored in non-obvious ways that makes it difficult to change anything about the internal representation of `bind_values`, and is extremely prone to bugs. So the goal is to move a lot of logic off of `Relation`, and into separate objects. This is not the same as what is currently done with `JoinDependency`, as `Relation` knows far too much about its internals, and vice versa. Instead these objects need to be black boxes that can have their implementations swapped easily. The end result will be two classes, `WhereClause` and `JoinClause` (`having` will just re-use `WhereClause`), and there will be a single method to access the bind values of a `Relation` which will be implemented as ``` join_clause.binds + where_clause.binds + having_clause.binds ``` This is the first step towards that refactoring, with the internal representation of where changed, and an intermediate representation of `where_values` and `bind_values` to let the refactoring take small steps. These will be removed shortly.
* Remove all cases of manuallly wrapping `Arel::Nodes::Quoted`Sean Griffin2014-12-291-6/+6
| | | | | | | | | | This is no longer required now that we are injecting a type caster object into the Arel table, with the exception of uniqueness validations. Since it calls `ConnectionAdapter#type_cast`, the value has already been cast for the database. We don't want Arel to attempt to cast it further, so we need to continue wrapping it in a quoted node. This can potentially go away when this validator is refactored to make better use of `where` or the predicate builder.
* Inform Arel that we don't need type casting in testsSean Griffin2014-12-261-6/+9
| | | | | | | Part of the larger refactoring to remove type casting from Arel. We can inform it that we already have the right type by wrapping the value in an `Arel::Nodes::Quoted`. This commit can be reverted when we have removed type casting from Arel in Rail 5.1
* Inject the `PredicateBuilder` into the `Relation` instanceSean Griffin2014-12-261-25/+25
| | | | | | | Construction of relations can be a hotspot, we don't want to create one of these in the constructor. This also allows us to do more expensive things in the predicate builder's constructor, since it's created once per AR::Base subclass
* Consolidate testing of update_all type castingSean Griffin2014-06-261-0/+28
| | | | | | | | | | | | | | We have several test cases on "tricky" types that are essentially testing that `update_all` goes through the same type casting behavior as a normal assignment + save. We recently had another case to add this test for another type in https://github.com/rails/rails/pull/12742. Rather than testing this separately for every type which is "tricky" when round tripping, let's instead have a fairly exhaustive test that ensures we're getting the correct values at every step for `update_all`. Given the structure of the code now, we can be confident that if the type is correct, and `update_all` is type casting correctly, we're going to get the right behavior for all types.
* Merge pull request #12829 from iantropov/issue_insert_via_hmt_scope_3548Rafael Mendonça França2014-04-101-0/+4
|\ | | | | | | | | | | | | Fix insertion of records for hmt association with scope Conflicts: activerecord/CHANGELOG.md
| * Fix insertion of records for hmt association with scope, fix #3548Ivan Antropov2013-11-171-0/+4
| |
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-06 01:45:25 +0000