aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
...
| * | | | | Delete `DateAndTime` method definition in rails that is compatible with ruby ↵soartec-lab2019-06-166-128/+112
|/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | definition Tests are also only on the `Time` class Update doc forgetting to erase when moved Update guide `Date` class to `Time` class and defined file Update guide correction omission
* | | | | Update default value of `variable_content_types` and ↵yuuji.yaginuma2019-06-161-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `content_types_to_serve_as_binary` [ci skip] Ref: bcf370d689673031073ba2ac5588afe41cc315c9, 06ab7b27ea1c1ab357085439abacdb464f6742bf.
* | | | | Merge pull request #36493 from kamipo/remove_unused_attributes_forRyuta Kamizono2019-06-164-70/+1
|\ \ \ \ \ | | | | | | | | | | | | Remove unused `Arel::Attributes.for`
| * | | | | Remove unused `Arel::Attributes.for`Ryuta Kamizono2019-06-154-70/+1
| | | | | | | | | | | | | | | | | | | | | | | | `Arel::Attributes.for` is no longer used since https://github.com/rails/arel/pull/196.
* | | | | | Merge pull request #36494 from ↵Ryuta Kamizono2019-06-161-3/+3
|\ \ \ \ \ \ | |/ / / / / |/| | | | | | | | | | | | | | | | | soartec-lab/fix/guide_delete_description_for_date_and_time Delete 'ruby' in the description of the method defined in rails [skip ci]
| * | | | | Delete 'ruby' in the description of the method defined in rails [skip ci]soartec-lab2019-06-161-3/+3
|/ / / / /
* | | | | Merge pull request #36483 from kamipo/no_allocation_to_sql_visitRyuta Kamizono2019-06-154-33/+34
|\ \ \ \ \ | | | | | | | | | | | | No allocation `Arel::Visitors::ToSql#visit`
| * | | | | No allocation `Arel::Visitors::ToSql#visit`Ryuta Kamizono2019-06-154-33/+34
|/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Each `visit o, collector` allocates one extra array due to receiving args by splat array. https://github.com/rails/rails/blob/2c3332cc4c0fa77dbe2e13e8a792f80fbd8f4ad3/activerecord/lib/arel/visitors/visitor.rb#L27-L29 Currently 1,000 times `User.where(id: 1).to_sql` allocates 13,000 arrays in `visitor.accept`. This avoids receiving args by splat array, it makes `visitor.accept` no array allocation. ```ruby ObjectSpace::AllocationTracer.setup(%i{path line type}) pp ObjectSpace::AllocationTracer.trace { 1_000.times { User.where(id: 1).to_sql } }.select { |k, _| k[2] == :T_ARRAY && k[0].end_with?("visitor.rb", "to_sql.rb") } ``` Before (2c3332cc4c0fa77dbe2e13e8a792f80fbd8f4ad3): ``` {["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 18, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb", 11, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/visitor.rb", 12, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 788, :T_ARRAY]=>[3000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 794, :T_ARRAY]=>[3000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 156, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 443, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 603, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0], ["~/rails/activerecord/lib/arel/visitors/to_sql.rb", 611, :T_ARRAY]=>[1000, 0, 0, 0, 0, 0]} ``` After (this change): ``` {} ```
* | | | | :golf:Akira Matsuda2019-06-151-6/+2
| | | | |
* | | | | Should find last created recordRyuta Kamizono2019-06-151-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Tables in tests are not always empty so `klass.first` does not always find last created record. Fixes #36479.
* | | | | Ensure to reset actually used `@connection.schema_migration`'s table nameRyuta Kamizono2019-06-151-4/+4
| | | | | | | | | | | | | | | | | | | | https://buildkite.com/rails/rails/builds/61744#f12cc6cf-7458-4131-917a-9735615f6259/999-1010
* | | | | Fix `test_schema_names` to include "hint_plan" schemaRyuta Kamizono2019-06-151-1/+5
| | | | |
* | | | | Merge pull request #36469 from ↵Eileen M. Uchitelle2019-06-148-49/+111
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | eileencodes/move-while_preventing_writes-to-handler Move while_preventing_writes from conn to handler
| * | | | | Move while_preventing_writes from conn to handlereileencodes2019-06-148-49/+111
|/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we put the `while_preventing_writes` on the connection then the middleware that sends reads to the primary and ensures they can't write will not work. The `while_preventing_writes` will only be applied to the connection which it's called on - which in the case of the middleware is Ar::Base. This worked fine if you called it directly like `OtherDbConn.connection.while_preventing_writes` but Rails didn't have a way of knowing you wanted to call it on all the connections. The change here moves the `while_preventing_writes` method from the connection to the handler so that it can block writes to all queries for that handler. This will apply to all the connections associated with that handler.
* | | | | Merge pull request #36488 from eileencodes/update-multi-db-docsEileen M. Uchitelle2019-06-141-1/+20
|\ \ \ \ \ | | | | | | | | | | | | Update multi-db docs
| * | | | | Update multi-db docseileencodes2019-06-141-1/+20
|/ / / / / | | | | | | | | | | | | | | | | | | | | | | | | | * Add note about schema cache * Add note about opening too many connections * Improve headers in caveats section
* | | | | Merge pull request #36439 from ↵Eileen M. Uchitelle2019-06-1417-150/+412
|\ \ \ \ \ | |_|_|/ / |/| | | | | | | | | | | | | | eileencodes/move-schema-migration-to-migration-context Move SchemaMigration to migration_context
| * | | | Move SchemaMigration to migration_contexteileencodes2019-06-1417-150/+412
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR moves the `schema_migration` to `migration_context` so that we can access the `schema_migration` per connection. This does not change behavior of the SchemaMigration if you are using one database. This also does not change behavior of any public APIs. `Migrator` is private as is `MigrationContext` so we can change these as needed. We now need to pass a `schema_migration` to `Migrator` so that we can run migrations on the right connection outside the context of a rake task. The bugs this fixes were discovered while debugging the issues around the SchemaCache on initialization with multiple database. It was clear that `get_all_versions` wouldn't work without these changes outside the context of a rake task (because in the rake task we establish a connection and change AR::Base.connection to the db we're running on). Because the `SchemaCache` relies on the `SchemaMigration` information we need to make sure we store it per-connection rather than on ActiveRecord::Base. [Eileen M. Uchitelle & Aaron Patterson]
* | | | | Merge pull request #36484 from ↵Ryuta Kamizono2019-06-141-3/+3
|\ \ \ \ \ | |/ / / / |/| | | | | | | | | | | | | | albertoalmagro/alberto/reverse-column-is-reversible [ci skip] Update docs as `remove_column` can be reversed
| * | | | [ci skip] Update docs as `remove_column` can be reversedAlberto Almagro2019-06-141-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | As `remove_column` can be reversed when a type is provided this example was not accurate anymore.
* | | | | Fix rubocop violationsRyuta Kamizono2019-06-142-3/+2
| |/ / / |/| | |
* | | | Merge pull request #36478 from kamipo/allocation_on_demand_in_transactionRyuta Kamizono2019-06-141-20/+35
|\ \ \ \ | | | | | | | | | | Allocation on demand in transactions
| * | | | Allocation on demand in transactionsRyuta Kamizono2019-06-141-20/+35
|/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently 1,000 transactions creates 10,000 objects regardless whether it is necessary or not. This makes allocation on demand in transactions, now 1,000 transactions creates required 5,000 objects only by default. ```ruby ObjectSpace::AllocationTracer.setup(%i{path line type}) pp ObjectSpace::AllocationTracer.trace { 1_000.times { User.create } }.select { |k, _| k[0].end_with?("transaction.rb") } ``` Before (95d038f): ``` {["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 209, :T_HASH]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 210, :T_OBJECT]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 210, :T_HASH]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 80, :T_OBJECT]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 8, :T_ARRAY]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 81, :T_ARRAY]=>[1000, 0, 715, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 289, :T_STRING]=>[1000, 0, 714, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 116, :T_ARRAY]=>[1000, 0, 714, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 120, :T_ARRAY]=>[1000, 0, 714, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 121, :T_HASH]=>[1000, 0, 714, 0, 1, 0]} ``` After (this change): ``` {["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 213, :T_HASH]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 214, :T_OBJECT]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 214, :T_HASH]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 81, :T_OBJECT]=>[1000, 0, 739, 0, 1, 0], ["~/rails/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb", 304, :T_STRING]=>[1000, 0, 738, 0, 1, 0]} ```
* | | | Merge pull request #36477 from albertoalmagro/alberto/button-to-default-pathRafael França2019-06-131-1/+1
|\ \ \ \ | | | | | | | | | | [ci skip] Use default path in button_to documentation
| * | | | [ci skip] Use default path in button_to documentationAlberto Almagro2019-06-131-1/+1
| |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is really a nit pick, but as this is the framework's documentation I think it should follow standards as many times as possible to avoid confusion in new users. If we were using `resources :articles` in routes. which is what scaffold adds, the generated helper would be `new_article_path` instead of `new_articles_path`.
* | | | Merge pull request #36437 from sudara/fix_programmatic_clicks_with_data_remoteGannon McGibbon2019-06-132-2/+16
|\ \ \ \ | |/ / / |/| | | Fix programmatic clicks with data-remote
| * | | Ensure non-mouse/programmatic clicks work with data-remoteSudara2019-06-132-2/+16
| | | |
* | | | Make ActiveRecord `ConnectionPool.connections` thread-safe. (#36473)jeffdoering2019-06-133-1/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Make ActiveRecord `ConnectionPool.connections` thread-safe. ConnectionPool documentation is clear on the need to synchronize access to @connections but also states that public methods do not require synchronization. Existing code exposed @connections directly via attr_reader. The fix uses synchronize() to lock @connections then returns a copy to the caller using Array.dup(). Includes comments on the connections method that thread-safe access to the connections array does not imply thread-safety of accessing methods on the actual connections. Adds a test-case that modifies the pool using a supported method in one thread while a second thread accesses pool.connections. The test fails without this patch. Fixes #36465. * Update activerecord/test/cases/connection_pool_test.rb [jeffdoering + Rafael Mendonça França]
* | | | Merge pull request #36466 from wbnns/update-missing-create-action-screenshotRafael França2019-06-131-0/+0
|\ \ \ \ | | | | | | | | | | images/getting_started: Update screenshot for missing action
| * | | | [ci skip] images/getting_started: Update screenshot for missing actionWill Binns2019-06-121-0/+0
| | | | |
* | | | | Merge pull request #36388 from joelhawksley/actionview-componentAaron Patterson2019-06-134-1/+72
|\ \ \ \ \ | | | | | | | | | | | | Introduce ActionView::Component
| * | | | | `RenderingHelper` supports rendering objects that `respond_to?` `:render_in`Joel Hawksley2019-06-124-1/+72
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Co-authored-by: Natasha Umer <natashau@github.com> Co-authored-by: Aaron Patterson <tenderlove@github.com> Co-authored-by: Shawn Allen <shawnbot@github.com> Co-authored-by: Emily Plummer <emplums@github.com> Co-authored-by: Diana Mounter <broccolini@github.com> Co-authored-by: John Hawthorn <jhawthorn@github.com> Co-authored-by: Nathan Herald <myobie@github.com> Co-authored-by: Zaid Zawaideh <zawaideh@github.com> Co-authored-by: Zach Ahn <engineering@zachahn.com>
| * | | | | Merge pull request #2 from rails/masterJoel Hawksley2019-06-0365-293/+486
| |\ \ \ \ \ | | | | | | | | | | | | | | merge master
* | | | | | | Ensure to reset migration version after testing migrationRyuta Kamizono2019-06-131-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "schema_migrations" table may be hard dropped before, so the reset migration version should be done in ensure block. https://buildkite.com/rails/rails/builds/61697#18d6f3ac-2257-4f4b-8efc-4010464c4d9a/999-1011
* | | | | | | Reset migration version before testing migrationRyuta Kamizono2019-06-131-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | https://buildkite.com/rails/rails/builds/61695#373bb1a7-677f-49ec-95e7-a92467fefd60/1076-1084
* | | | | | | Avoid implicit rollback when testing migrationRyuta Kamizono2019-06-131-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "schema_migrations" is hard dropped by some existing tests, so testing migration in using transactional tests may cause implicit creation and rollback "schema_migrations" table, it makes migration tests flaky. https://buildkite.com/rails/rails/builds/61692#42383249-30be-4508-b1fb-a7bb27600c8e/999-1010 https://buildkite.com/rails/rails/builds/61694#6e462ad3-41d8-4e26-95ce-728495b0ac64/999-1010
* | | | | | | Merge pull request #36472 from kamipo/empty_line_only_before_access_modifierRyuta Kamizono2019-06-13434-514/+8
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | Enable `Layout/EmptyLinesAroundAccessModifier` cop
| * | | | | | | Enable `Layout/EmptyLinesAroundAccessModifier` copRyuta Kamizono2019-06-13434-514/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | | | | | | | Remove duplicated `table_exists?`Ryuta Kamizono2019-06-132-8/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `table_exists?` is already exist in `ModelSchema`. https://github.com/rails/rails/blob/5cab344494c340ea82a35b46efa06b94f0b7730b/activerecord/lib/active_record/model_schema.rb#L339-L341
* | | | | | | | Don't `drop_table` before schema cache testsRyuta Kamizono2019-06-131-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since 5cab34449, `drop_table` clears schema cache.
* | | | | | | | Clear schema cache when a table is created/dropped/renamedRyuta Kamizono2019-06-136-4/+18
|/ / / / / / / | | | | | | | | | | | | | | | | | | | | | Otherwise `Model.table_exists?` returns the staled cache result.
* | | | | | | Merge pull request #36470 from lucasprag/activesupport/missing-require-digestRafael França2019-06-121-0/+3
|\ \ \ \ \ \ \ | |_|_|/ / / / |/| | | | | | Add missing file to require digest/uuid on active_support core ext
| * | | | | | Add missing file to require digest/uuid on active_support core extensionsLucas Arantes2019-06-121-0/+3
|/ / / / / /
* | | | | | Merge pull request #36447 from holyketzer/masterRafael França2019-06-111-1/+1
|\ \ \ \ \ \ | | | | | | | | | | | | | | ActionCable: optimise logger.debug calling
| * | | | | | ActionCable: optimize logger.debug callingAlex Emelyanov2019-06-081-1/+1
| | | | | | |
* | | | | | | Merge pull request #36440 from malept/multi-db-abort_if_pending_migrations-taskEileen M. Uchitelle2019-06-113-1/+55
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | Add support for multiple databases to `rails db:abort_if_pending_migrations`
| * | | | | | | Convert the db:abort_if_pending_migrations task to be multi-DB awareMark Lee2019-06-103-1/+55
| | | | | | | |
* | | | | | | | Merge pull request #36448 from ↵Ryuta Kamizono2019-06-117-42/+67
|\ \ \ \ \ \ \ \ | |/ / / / / / / |/| | | | | | | | | | | | | | | | | | | | | | | kamipo/allow_column_name_with_simple_function_call Allow column name with function (e.g. `length(title)`) as safe SQL string
| * | | | | | | Allow column name with function (e.g. `length(title)`) as safe SQL stringRyuta Kamizono2019-06-107-42/+67
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | | | | | | | All modern adapters returns a numeric value as the result of numeric calculationRyuta Kamizono2019-06-112-10/+2
| | | | | | | |