aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/test
Commit message (Collapse)AuthorAgeFilesLines
* Merge pull request #22125 from k0kubun/left_joinsSean Griffin2015-10-302-1/+6
|\ | | | | Alias left_joins to left_outer_joins
| * Alias left_joins to left_outer_joinsTakashi Kokubun2015-10-312-1/+6
| |
* | don't sleep in testsAaron Patterson2015-10-301-10/+6
| | | | | | | | | | we should be using a countdown latch instead of rolling our own busy-loop.
* | Merge pull request #22122 from ↵Sean Griffin2015-10-304-0/+199
|\ \ | | | | | | | | | | | | samphilipd/sam/manual_locking_on_schema_migrations Make migrations concurrent safe (using advisory locks)
| * | Use advisory locks to prevent concurrent migrationsSam Davies2015-10-304-0/+199
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - Addresses issue #22092 - Works on Postgres and MySQL - Uses advisory locks because of two important properties: 1. The can be obtained outside of the context of a transaction 2. They are automatically released when the session ends, so if a migration process crashed for whatever reason the lock is not left open perpetually - Adds get_advisory_lock and release_advisory_lock methods to database adapters - Attempting to run a migration while another one is in process will raise a ConcurrentMigrationError instead of attempting to run in parallel with undefined behavior. This could be rescued and the migration could exit cleanly instead. Perhaps as a configuration option? Technical Notes ============== The Migrator uses generate_migrator_advisory_lock_key to build the key for the lock. In order to be compatible across multiple adapters there are some constraints on this key. - Postgres limits us to 64 bit signed integers - MySQL advisory locks are server-wide so we have to scope to the database - To fulfil these requirements we use a Migrator salt (a randomly chosen signed integer with max length of 31 bits) that identifies the Rails migration process as the owner of the lock. We multiply this salt with a CRC32 unsigned integer hash of the database name to get a signed 64 bit integer that can also be converted to a string to act as a lock key in MySQL databases. - It is important for subsequent versions of the Migrator to use the same salt, otherwise different versions of the Migrator will not see each other's locks.
* | | Fix test failures caused by #12071Sean Griffin2015-10-301-10/+10
| |/ |/| | | | | | | | | This assumes only one query was ever executed, but it appears to sometimes be loading schema information. We can just look at the array of queries, rather than the "first" one that was run
* | Merge pull request #12071 from Crunch09/outer_joinsSean Griffin2015-10-301-0/+74
|\ \ | | | | | | | | | added ActiveRecord::Relation#outer_joins
| * | added ActiveRecord::Relation#left_outer_joinsFlorian Thomas2015-05-191-0/+74
| | | | | | | | | | | | | | | | | | Example: User.left_outer_joins(:posts) => SELECT "users".* FROM "users" LEFT OUTER JOIN "posts" ON "posts"."user_id" = "users"."id"
* | | Ensure `has_and_belongs_to_many` works with `belongs_to_required_by_default`Sean Griffin2015-10-292-0/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Before this commit, if `ActiveRecord::Base.belongs_to_required_by_default` is set to `true`, then creating a record through `has_and_belongs_to_many` fails with the cryptic error message `Left side must exist`. This is because `inverse_of` isn't working properly in this case, presumably since we're doing trickery with anonymous classes in the middle. Rather than following this rabbit hole to try and get `inverse_of` to work in a case that we know is not publicly supported, we can just turn off this validation to match the behavior of 4.2 and earlier.
* | | Merge pull request #13008 from ktheory/sanitize_orderSean Griffin2015-10-291-0/+5
|\ \ \ | | | | | | | | | | | | Support SQL sanitization in AR::QueryMethods#order
| * | | Support SQL sanitization in AR::QueryMethods#orderAaron Suggs2013-11-221-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add support for sanitizing arrays in SQL ORDER clauses. This is useful when using MySQL `ORDER BY FIELD()` to return records in a predetermined way. ```ruby Tag.order(['field(id, ?', [1,3,2]].to_sql # => SELECT "tags".* FROM "tags" ORDER BY field(id, 1,3,2) ``` Prior to this, developers must be careful to sanitize `#order` arguments themselves.
* | | | Never pass `nil` to `order`Sean Griffin2015-10-291-6/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is part of a refactoring to make it easier to allow `order` to use sanitize like just about everything else on relation. The deleted test doesn't give any reasoning as to why passing `nil` to `order` needs to be supported, and it's rather nonsensical. I can almost see allowing an empty string being passed (though I'm tempted to just disallow it...)
* | | | Merge pull request #22106 from jamis/fix_default_fixture_class_namesYves Senn2015-10-291-0/+14
|\ \ \ \ | | | | | | | | | | default value for FixtureSet.fixture_class_names ought to be a Class
| * | | | FixtureSet.fixture_class_names should have no default valueJamis Buck2015-10-291-0/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Look at `TestFixtures.set_fixture_class`. As documented, it accepts a mapping of fixture identifiers (string or symbol) to Classes (the model classes that implement the named fixture). Look now at the initialization of `TestFixtures.fixture_class_names`. It defines a Hash, which will return a string by default (where the string is the estimated class name of the given fixture identifier). Now look at TestFixtures.load_fixtures. It calls `FixtureSet.create_fixtures`, passing in the mapping of `fixture_class_names`. Following this on to `FixtureSet.create_fixtures`, this instantiates a `FixtureSet::ClassCache`, passing in the map of class names. `ClassCache`, in turn, calls `insert_class` for each value in the cache. (Recall that `set_fixture_class` puts Class objects in there, while the default proc for the mapping puts String objects.) Look finally at `insert_class`. If the value is present, it checks to see if the value is a subclass of `AR::Base`. Fair enough...but wait! What if the value is a String? You get an exception, because a String instance cannot be compared with a Class. Judging from the implementation, it seems like the expected behavior here is for `fixture_class_names` to have no default proc. Look-ups are supposed to happen via `ClassCache`, with `fixture_class_names` existing solely as a repository for explicitly-registered class mappings. That is what this change does.
* | | | | Revert "Revert "Merge pull request #22026 from ↵Yves Senn2015-10-291-0/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | akihiro17/fix-preload-association"" This reverts commit 5243946017d09afff4d70d273b0fcdfd41a4b22a. This fixes an issue with the build where tests would fail on mysql and postgresql due to different ordering.
* | | | | Revert "Merge pull request #22026 from akihiro17/fix-preload-association"Sean Griffin2015-10-291-18/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 6dc6a0b17cfaf7cb6aa2b1c163b6ca141b538a8e, reversing changes made to ec94f00ba3cf250eb54fc5b7a5e3ed4b90164f34. This pull request broke the build.
* | | | | Merge pull request #19501 from ccutrer/dry_sti_subclass_finding2Sean Griffin2015-10-295-3/+12
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | DRY up STI subclass logic
| * | | | | Fix test failures caused by #19501Sean Griffin2015-10-292-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The first one is quite straightforward. We want to give the proper error message in the case where a top level constant exists, but we're looking for a nested one. We just need to port over the change to use `subclass.name` into these changes. The second set of failures, which are only present in the mysql adapter tests, are stranger to me. The failure occurs because we were previously comparing `subclass.name == self.name` instead of `subclass == self`. However, I don't think that we need to support creating anonymous classes which share a table with a class that uses STI, overrides `name` to return the same name as athe class that we have no other relationship with, when not assigned to a constant so it could never be used anyway... The commits around why that exist give no context, and I think they're just poorly written tests (WTF does `test_schema` mean anyway, and why does calling `.first` on some anonymous class test it?). We'll just disable STI on that class.
| * | | | | Fix merge conflicts from #19501Sean Griffin2015-10-293-3/+10
| |\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I'm making this commit separately because this has failing tests and style nitpicks that I'd like to make as individual commits, to make the changes I'm making explicit. We still want a single merge commit at the end, however.
| | * | | | | DRY up STI subclass logicCody Cutrer2015-03-243-3/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | the newer method used for discriminating new records did not use the older and more robust method used for instantiating existing records, but did have a better post-check to ensure the sublass was in the hierarchy. so move the descendants check to find_sti_class, and then simply call find_sti_class from subclass_from_attributes now with fixed specs
* | | | | | | Set `scope.reordering_value` to `true` if :reordering values are specifiedakihiro172015-10-301-0/+18
|/ / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We should call `scope.order!` and set `scope.reordering_value` to `true` if :reordering values are specified Fixes #21886
* | | | | | tests, no every adapter supports "connection.version"Yves Senn2015-10-292-22/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This solves the following issue: ``` $ bin/test Using sqlite3 /Users/senny/Projects/rails/activerecord/test/cases/adapters/mysql2/sp_test.rb:16:in `<class:Mysql2StoredProcedureTest>': undefined method `version' for #<ActiveRecord::ConnectionAdapters::SQLite3Adapter:0x007f8bab4b5b70> (NoMethodError) from /Users/senny/Projects/rails/activerecord/test/cases/adapters/mysql2/sp_test.rb:5:in `<top (required)>' from /Users/senny/Projects/rails/activesupport/lib/active_support/dependencies.rb:302:in `require' from /Users/senny/Projects/rails/activesupport/lib/active_support/dependencies.rb:302:in `block in require' from /Users/senny/Projects/rails/activesupport/lib/active_support/dependencies.rb:268:in `load_dependency' from /Users/senny/Projects/rails/activesupport/lib/active_support/dependencies.rb:302:in `require' from /Users/senny/Projects/rails/railties/lib/rails/test_unit/test_requirer.rb:11:in `block in require_files' from /Users/senny/Projects/rails/railties/lib/rails/test_unit/test_requirer.rb:10:in `each' from /Users/senny/Projects/rails/railties/lib/rails/test_unit/test_requirer.rb:10:in `require_files' from /Users/senny/Projects/rails/railties/lib/rails/test_unit/minitest_plugin.rb:69:in `plugin_rails_init' from /Users/senny/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/minitest-5.3.3/lib/minitest.rb:73:in `block in init_plugins' from /Users/senny/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/minitest-5.3.3/lib/minitest.rb:71:in `each' from /Users/senny/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/minitest-5.3.3/lib/minitest.rb:71:in `init_plugins' from /Users/senny/.rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/minitest-5.3.3/lib/minitest.rb:122:in `run' from bin/test:19:in `<main>' ```
* | | | | | Merge pull request #18548 from ↵Sean Griffin2015-10-285-0/+95
|\ \ \ \ \ \ | |_|/ / / / |/| | | | | | | | | | | | | | | | | | | | | | | sebjacobs/support-bidirectional-destroy-dependencies Add support for bidirectional destroy dependencies
| * | | | | Add support for bidirectional destroy dependenciesSeb Jacobs2015-01-165-0/+95
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Prior to this commit if you defined a bidirectional relationship between two models with destroy dependencies on both sides, a call to `destroy` would result in an infinite callback loop. Take the following relationship. class Content < ActiveRecord::Base has_one :content_position, dependent: :destroy end class ContentPosition < ActiveRecord::Base belongs_to :content, dependent: :destroy end Calling `Content#destroy` or `ContentPosition#destroy` would result in an infinite callback loop. This commit changes the behaviour of `ActiveRecord::Callbacks#destroy` so that it guards against subsequent callbacks. Thanks to @zetter for demonstrating the issue with failing tests[1]. [1] rails#13609
* | | | | | Merge pull request #18383 from ↵Rafael Mendonça França2015-10-271-0/+19
|\ \ \ \ \ \ | |_|_|_|_|/ |/| | | | | | | | | | | | | | | | | | | | | | | scambra/habtm-with-where-includes-16032-for-master Includes HABTM returns correct size now
| * | | | | Includes HABTM returns correct size now. It's caused by the join dependencySergio Cambra2015-01-071-0/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | only instantiates one HABTM object because the join table hasn't a primary key. Updated commit from @bigxiang commit dbaa837 Fixes #16032. Examples: before: Project.first.salaried_developers.size # => 3 Project.includes(:salaried_developers).first.salaried_developers.size # => 1 after: Project.first.salaried_developers.size # => 3 Project.includes(:salaried_developers).first.salaried_developers.size # => 3
* | | | | | Revert "Merge pull request #21994 from mtodd/inherit-scopes"Rafael Mendonça França2015-10-272-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 60c9701269f5b412849f1a507df61ba4735914d7, reversing changes made to 6a25202d9ea3b4a7c9f2d6154b97cf8ba58403db. Reason: Broken build
* | | | | | Make inherited scope test failMatt Todd2015-10-262-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This triggers the JoinDependency work to reflect on the associations and trigger an error as follows: ActiveRecord::ConfigurationError: Association named 'account' was not found on Company; perhaps you misspelled it? Fix Company.of_first_firm joins association name Should be `Company.joins(:accounts)` not `Company.joins(:account)`. Do the same for Client.of_first_firm
* | | | | | Merge pull request #19686 from tsun1215/index_errorsSean Griffin2015-10-264-0/+53
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Errors can be indexed with nested attributes Close #8638
| * | | | | | Errors can be indexed with nested attributesMichael Probber2015-04-174-0/+53
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `has_many` can now take `index_errors: true` as an option. When this is enabled, errors for nested models will be returned alongside an index, as opposed to just the nested model name. This option can also be enabled (or disabled) globally through `ActiveRecord::Base.index_nested_attribute_errors` E.X. ```ruby class Guitar < ActiveRecord::Base has_many :tuning_pegs accepts_nested_attributes_for :tuning_pegs end class TuningPeg < ActiveRecord::Base belongs_to :guitar validates_numericality_of :pitch end ``` - Old style - `guitar.errors["tuning_pegs.pitch"] = ["is not a number"]` - New style (if defined globally, or set in has_many_relationship) - `guitar.errors["tuning_pegs[1].pitch"] = ["is not a number"]` [Michael Probber, Terence Sun]
* | | | | | | Merge pull request #19924 from iamvery/db-tasks-exit-statusSean Griffin2015-10-262-2/+2
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | Explicitly exit with status "1" for create and drop failures
| * | | | | | | Exit with non-zero status when db:create failsJay Hayes2015-10-202-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * If the create task fails for a reason other than the database already existing, processing should end. This is indicated by a non-zero exit status. * Since the backtrace is already printed to screen, we forgo printing it again by using an explicit call to `exit`. * :warning: This modifies the behavior of the db:create task slightly in that the stack trace is no longer printed by default. If the `--trace` option is used, it will print the trace _after_ the error message.
* | | | | | | | Tweaked wording used in some tests.Sebastian McKenzie2015-10-251-4/+4
| | | | | | | |
* | | | | | | | Merge pull request #21950 from ↵Andrew White2015-10-222-1/+5
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | rafaelsales/issues/21922-fix-ar-group-by-attribute-lookup Fix generated projection fields in group by query
| * | | | | | | | Refactor Calculations#execute_grouped_calculation and clean AR test caseRafael Sales2015-10-221-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * When tried to use `Company#accounts` test/models/company.rb I got: ``` ActiveRecord::StatementInvalid: SQLite3::SQLException: no such column: accounts.company_id: SELECT COUNT(*) AS count_all, "companies"."firm_id" AS companies_firm_id FROM "companies" INNER JOIN "accounts" ON "accounts"."company_id" = "companies"."id" GROUP BY "companies"."firm_id" ``` * The refactor on Calculations class was just to simplify the code
| * | | | | | | | Fix generated projection fields in group by queryRafael Sales2015-10-221-0/+5
| |/ / / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Closes #21922 Let `Book(id, author_id)`, `Photo(id, book_id, author_id)` and `Author(id)` Running `Book.group(:author_id).joins(:photos).count` will produce: * Rails 4.2 - conflicts `author_id` in both projection and group by: ```sql SELECT COUNT(*) AS count_all, author_id AS author_id FROM "books" INNER JOIN "photos" ON "photos"."book_id" = "books"."id" GROUP BY author_id ``` * Master (9d02a25) - conflicts `author_id` only in projection: ```sql SELECT COUNT(*) AS count_all, author_id AS author_id FROM "books" INNER JOIN "photos" ON "photos"."book_id" = "books"."id" GROUP BY "books"."author_id" ``` * With this fix: ```sql SELECT COUNT(*) AS count_all, "books"."author_id" AS books_author_id FROM "books" INNER JOIN "photos" ON "photos"."book_id" = "books"."id" GROUP BY "books"."author_id" ```
* / / / / / / / Remove `#tables` extra args againRyuta Kamizono2015-10-222-4/+3
|/ / / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This issue was resolved by #21687 already. But re-add args by #18856. `#tables` extra args was only using by `#table_exists?`. This is for internal API. This commit will remove these extra args again.
* | | | | | | Fix merge conflicts for #19938Sean Griffin2015-10-205-0/+24
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a separate commit, as it is not just a changelog conflict. Want to point out the changes in the code
| * | | | | | | Fix for activerecord join dependency instantiate bugMehmet Emin İNAÇ2015-05-045-0/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | use only object_id instead parent class and parent id test cases assert_equal use table name in references fix minor problems
* | | | | | | | Merge pull request #20653 from repinel/allow-arel-select-countSean Griffin2015-10-201-0/+52
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | Allow select using Arel and perform a count
| * | | | | | | | Allow select with Arel and count as well as calculations with ArelRoque Pinel2015-06-301-0/+52
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It allows a query like `User.select(:name).count` to be written using Arel as `User.select(User.arel_table[:name]).count`. It exposes the calculations API to accept Arel nodes: `User.count(User.arel_table[:name])`, `User.sum(User.arel_table[:id])`, `Account.average(Account.arel_table[:credit_limit])`, `Account.maximum(Account.arel_table[:credit_limit])` and `Account.minimum(Account.arel_table[:credit_limit])`.
* | | | | | | | | Merge pull request #20957 from akihiro17/find-by-issueSean Griffin2015-10-201-0/+6
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix find_by with association subquery issue
| * | | | | | | | | Don't cache arguments in #find_by if they are an ActiveRecord::Relationakihiro172015-10-061-0/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In this commit, find_by doesn't cache arguments so that find_by with association subquery works correctly. Fixes #20817
* | | | | | | | | | Fix test failures caused by d99db6b8b3e4Sean Griffin2015-10-201-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I messed up the merge conflict, and accidentally removed a schema query that needed to be ignored.
* | | | | | | | | | Qualify column names in calculationSoutaro Matsumoto2015-10-201-0/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Column names inserted via `group` have to be qualified with table name.
* | | | | | | | | | Fix merge conflicts for #18856Sean Griffin2015-10-201-1/+1
|\ \ \ \ \ \ \ \ \ \
| * | | | | | | | | | Match table names exactly on MySQLMatt Jones2015-02-083-15/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `SHOW TABLES LIKE` command accepts metacharacters `%` and `_` in potentially unexpected ways. This can be avoided by querying `information_schema.tables` directly. Fixes #17897
* | | | | | | | | | | Merge pull request #21883 from tarzan/cache-key-too-preciseSean Griffin2015-10-203-6/+39
|\ \ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | Fix precision on cache_key
| * | | | | | | | | | | fixes #21815Maarten Jacobs2015-10-163-6/+39
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The default timestamp used for AR is `updated_at` in nanoseconds! (:nsec) This causes issues on any machine that runs an OS that supports nanoseconds timestamps, i.e. not-OS X, where the cache_key of the record persisted in the database (milliseconds precision) is out-of-sync with the cache_key in the ruby VM. This commit adds: A test that shows the issue, it can be found in the separate file `cache_key_test.rb`, because - model couldn't be defined inline - transactional testing needed to be turned off to get it to pass the MySQL tests This seemed cleaner than putting it in an existing testcase file. It adds :usec as a dateformat that calculates datetime in microseconds It sets precision of cache_key to :usec instead of :nsec, as no db supports nsec precision on timestamps
* | | | | | | | | | | | Do not cache prepared statements that are unlikely to have cache hitsSean Griffin2015-10-202-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Prior to this commit, Rails makes no differentiation between whether a query uses bind parameters, and whether or not we cache that query as a prepared statement. This leads to the cache populating extremely fast in some cases, with the statements never being reused. In particular, the two problematic cases are `where(foo: [1, 2, 3])` and `where("foo = ?", 1)`. In both cases we'll end up quoting the values rather than using a bind param, causing a cache entry for every value ever used in that query. It was noted that we can probably eventually change `where("foo = ?", 1)` to use a bind param, which would resolve that case. Additionally, on PG we can change our generated query to be `WHERE foo = ANY($1)`, and pass an array for the bind param. I hope to accomplish both in the future. For SQLite and MySQL, we still end up preparing the statements anyway, we just don't cache it. The statement will be cleaned up after it is executed. On postgres, we skip the prepare step entirely, as an API is provided to execute with bind params without preparing the statement. I'm not 100% happy on the way this ended up being structured. I was hoping to use a decorator on the visitor, rather than mixing a module into the object, but the way Arel has it's visitor pattern set up makes it very difficult to extend without inheritance. I'd like to remove the duplication from the various places that are extending it, but that'll require a larger restructuring of that initialization logic. I'm going to take another look at the structure of it soon. This changes the signature of one of the adapter's internals, and will require downstream changes from third party adapters. I'm not too worried about this, as worst case they can simply add the parameter and always ignore it, and just keep their previous behavior. Fixes #21992.