aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
Commit message (Collapse)AuthorAgeFilesLines
...
* | | | | | Merge pull request #31710 from eugeneius/indestructible_through_recordRyuta Kamizono2018-01-226-5/+24
|\ \ \ \ \ \ | | | | | | | | | | | | | | Don't update counter cache when through record was not destroyed
| * | | | | | Don't update counter cache when through record was not destroyedEugene Kenny2018-01-146-5/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When removing a record from a has many through association, the counter cache was being updated even if the through record halted the callback chain and prevented itself from being destroyed.
* | | | | | | PERF: Recover `changes_applied` performance (#31698)Ryuta Kamizono2018-01-222-5/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | #30985 caused `object.save` performance regression since calling `changes` in `changes_applied` is very slow. We don't need to call the expensive method in `changes_applied` as long as `@attributes` is tracked by mutation tracker. https://gist.github.com/kamipo/1a9f4f3891803b914fc72ede98268aa2 Before: ``` Warming up -------------------------------------- create_string_columns 73.000 i/100ms Calculating ------------------------------------- create_string_columns 722.256 (± 5.8%) i/s - 3.650k in 5.073031s ``` After: ``` Warming up -------------------------------------- create_string_columns 96.000 i/100ms Calculating ------------------------------------- create_string_columns 950.224 (± 7.7%) i/s - 4.800k in 5.084837s ```
* | | | | | | Merge pull request #31732 from ↵Matthew Draper2018-01-191-1/+1
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | koic/enable_autocorrect_for_lint_end_alignment_cop Enable autocorrect for `Lint/EndAlignment` cop
| * | | | | | | Enable autocorrect for `Lint/EndAlignment` copKoichi ITO2018-01-181-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ### Summary This PR changes .rubocop.yml. Regarding the code using `if ... else ... end`, I think the coding style that Rails expects is as follows. ```ruby var = if cond a else b end ``` However, the current .rubocop.yml setting does not offense for the following code. ```ruby var = if cond a else b end ``` I think that the above code expects offense to be warned. Moreover, the layout by autocorrect is unnatural. ```ruby var = if cond a else b end ``` This PR adds a setting to .rubocop.yml to make an offense warning and autocorrect as expected by the coding style. And this change also fixes `case ... when ... end` together. Also this PR itself is an example that arranges the layout using `rubocop -a`. ### Other Information Autocorrect of `Lint/EndAlignment` cop is `false` by default. https://github.com/bbatsov/rubocop/blob/v0.51.0/config/default.yml#L1443 This PR changes this value to `true`. Also this PR has changed it together as it is necessary to enable `Layout/ElseAlignment` cop to make this behavior.
* | | | | | | | Remove unused `connection` argument from `MigrationContext#current_version`Ryuta Kamizono2018-01-191-3/+3
| | | | | | | |
* | | | | | | | Remove unused `migration_context` in `DatabaseTasks`Ryuta Kamizono2018-01-191-4/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This was added in #31727, but it is unused.
* | | | | | | | Restore `test_migrations_status_with_schema_define_in_subdirectories`Ryuta Kamizono2018-01-191-7/+11
| |/ / / / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This test case which was added in #28287 checks that `ActiveRecord::Schema.define` loads migration versions in subdirectories. It should be kept it as it was.
* | | | | | | Merge pull request #31663 from Edouard-chin/remove-without-sql-modeRafael França2018-01-181-0/+22
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | Added a test around `NO_AUTO_VALUE_ON_ZERO`:
| * | | | | | | Added a test around `NO_AUTO_VALUE_ON_ZERO`:Edouard CHIN2018-01-171-0/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | - The mysql `NO_AUTO_VALUE_ON_ZERO` mode should be disabled when inserting fixtures in bulk, this PR adds a test to make sure we don't remove it by mistake - If we live this mode enabled, a statement like this wouldn't work and a `Duplicate entry '0' for key 'PRIMARY'` error will be raised. That's because `DEFAULT` on auto_increment will return 0 ```sql INSERT INTO `aircraft` (`id`, `name`, `wheels_count`) VALUES (DEFAULT, 'first', 2), (DEFAULT, 'second', 3) ```
* | | | | | | | Use selected_migrations if block_given?oz2018-01-181-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This slipped in as part of a2827ec9811b5012e8e366011fd44c8eb53fc714.
* | | | | | | | Refactor migration to move migrations paths to connectioneileencodes2018-01-1815-305/+402
| |/ / / / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rails has some support for multiple databases but it can be hard to handle migrations with those. The easiest way to implement multiple databases is to contain migrations into their own folder ("db/migrate" for the primary db and "db/seconddb_migrate" for the second db). Without this you would need to write code that allowed you to switch connections in migrations. I can tell you from experience that is not a fun way to implement multiple databases. This refactoring is a pre-requisite for implementing other features related to parallel testing and improved handling for multiple databases. The refactoring here moves the class methods from the `Migrator` class into it's own new class `MigrationContext`. The goal was to move the `migrations_paths` method off of the `Migrator` class and onto the connection. This allows users to do the following in their `database.yml`: ``` development: adapter: mysql2 username: root password: development_seconddb: adapter: mysql2 username: root password: migrations_paths: "db/second_db_migrate" ``` Migrations for the `seconddb` can now be store in the `db/second_db_migrate` directory. Migrations for the primary database are stored in `db/migrate`". The refactoring here drastically reduces the internal API for migrations since we don't need to pass `migrations_paths` around to every single method. Additionally this change does not require any Rails applications to make changes unless they want to use the new public API. All of the class methods from the `Migrator` class were `nodoc`'d except for the `migrations_paths` and `migrations_path` getter/setters respectively.
* | | | | | | Fix relation merger issue with `left_outer_joins`Mehmet Emin INAC2018-01-154-1/+37
| | | | | | |
* | | | | | | Don't allow destroyed object mutation after `save` or `save!` is calledRyuta Kamizono2018-01-153-2/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently `object.save` will unfreeze the object, due to `changes_applied` replaces frozen `@attributes` to new `@attributes`. Since originally destroyed objects are not allowed to be mutated, `save` and `save!` should not return success in that case. Fixes #28563.
* | | | | | | `create_database` should not add default charset when `collation` is givenRyuta Kamizono2018-01-152-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If `collation` is given without `charset`, it may generate invalid SQL. For example `create_database(:matt_aimonetti, collation: "utf8mb4_bin")`: ``` > CREATE DATABASE `matt_aimonetti` DEFAULT CHARACTER SET `utf8` COLLATE `utf8mb4_bin`; ERROR 1253 (42000): COLLATION 'utf8mb4_bin' is not valid for CHARACTER SET 'utf8' ``` In MySQL, charset is used to find the default collation. If `collation` is given explicitly, it is not necessary to give extra charset.
* | | | | | | Merge pull request #28313 from sandrew/masterRyuta Kamizono2018-01-152-2/+15
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | Allow unscoping of left_outer_joins
| * | | | | | | Allow unscoping of left_outer_joinsAndrei Shaidurov2017-03-061-1/+1
| | | | | | | |
* | | | | | | | Don't need to pass `manager` to `convert_join_strings_to_ast`Ryuta Kamizono2018-01-141-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Because `Relation` already have Arel `table`.
* | | | | | | | Don't pass garbage args to alias trackerRyuta Kamizono2018-01-142-11/+4
| |_|_|_|_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | | | | | | Merge pull request #31671 from larskanis/pg-1.0Rafael Mendonça França2018-01-111-1/+1
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | PostgreSQL: Allow pg-1.0 gem to be used with ActiveRecord
| * | | | | | | PostgreSQL: Allow pg-1.0 gem to be used with ActiveRecordLars Kanis2018-01-101-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | pg-1.0.0 is just released and most Gemfiles don't restrict it's version. But the version is checked when connecting to the database, which leads to the following error: Gem::LoadError: can't activate pg (~> 0.18), already activated pg-1.0.0 See also this pg issue: https://bitbucket.org/ged/ruby-pg/issues/270/pg-100-x64-mingw32-rails-server-not-start Preparation for pg-1.0 was done in commit f28a331023fab, but the pg version constraint was not yet relaxed.
* | | | | | | | Merge pull request #23146 from piotrj/issue_18424Ryuta Kamizono2018-01-114-0/+29
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | When deleting through records, take into account association conditions
| * | | | | | | | When deleting through records, take into account association conditionsPiotr Jakubowski2016-05-043-8/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fixes #18424. When deleting through records, it didn't take into account the conditions that may have been affecting join model table, but was defined in association definition.
* | | | | | | | | Remove the deprecated `:conditions` option in ↵Ryuta Kamizono2018-01-111-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `INVALID_AUTOMATIC_INVERSE_OPTIONS` This option was moved into active_record_deprecated_finders in ac4d101.
* | | | | | | | | Use `apply_join_dependency` instead of meaningless named ↵Ryuta Kamizono2018-01-112-26/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `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-112-13/+16
| |/ / / / / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `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.
* | | | | | | | [ci skip] setting :inverse_of works with :as, :through and :polymorphic ↵Bart de Water2018-01-101-12/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | options, and is needed for bi-directionality with a scope [ci skip] Remove :conditions opion from association basics guide This got replaced by scopes.
* | | | | | | | Fix `stale_state` for nested `has_many :through` associationsRyuta Kamizono2018-01-103-22/+41
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Need reloading when through record has replaced.
* | | | | | | | Merge pull request #16314 from ↵Ryuta Kamizono2018-01-104-1/+76
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | zoltankiss/allow-nested-has-many-associations-on-unpersisted-parent-instances fix nested `has many :through` associations on unpersisted parent instances
| * | | | | | | | Fix nested `has many :through` associations on unpersisted instancesZoltan Kiss2015-03-263-1/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fixes: #16313
* | | | | | | | | Bring back passing single record support for `Preloader`Ryuta Kamizono2018-01-101-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I removed redundant `Array.wrap(records)` since `Preloader` is nodoc class and Active Record always pass `records` as an array to `Preloader`. But if users relies on that behavior, it is not worth dropping its behavior. Fixes #31661.
* | | | | | | | | resolve inconsistencies between first and to_a.first with limitBrian Christian2018-01-093-1/+31
| | | | | | | | |
* | | | | | | | | Fix deleting through records when using has_many through with `source_type`Ryuta Kamizono2018-01-082-20/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently deleting through records doesn't respect `source_type`. It should not be ignored in that case. Related #23209. Fixes #24116.
* | | | | | | | | Simply use `scope.delete_all` instead of constructing delete managerRyuta Kamizono2018-01-071-8/+1
| | | | | | | | |
* | | | | | | | | 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-072-9/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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-072-1/+6
| | | | | | | | |
* | | | | | | | | Revert commit 4ec5b0d6b4d8a57e034b1014942356e95caf47aa in favor of #28379Ryuta Kamizono2018-01-072-9/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 4ec5b0d was for fixing the regression #18787, but #28379 fixes #18787 as well. So 4ec5b0d is no longer necessary.
* | | | | | | | | Remove passing argument to singular and collection association readersRyuta Kamizono2018-01-051-2/+2
| |_|_|/ / / / / |/| | | | | | | | | | | | | | | | | | | | | | | Follow up of 09cac8c67afdc4b2a1c6ae07931ddc082629b277.
* | | | | | | | Partial revert the changing default value of `readonly_value`Ryuta Kamizono2018-01-052-15/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a partial revert of #26182. There is no reason to change the default value.
* | | | | | | | Fix newly added reflection order when redefining associationRyuta Kamizono2018-01-043-4/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently reflections keeps the order when first added even if when redefining association. As a result of the order, redefining through association which use newly added association will raise `HasManyThroughOrderError`. We need to redefine reflection order as well when redefining association. Fixes #31068.
* | | | | | | | Deprecate `valid_alter_table_type?` in sqlite3 adapterRyuta Kamizono2018-01-042-7/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This method which is used only in the internal was introduced in ac384820 and was renamed in #17579. It does not need to be exposed.
* | | | | | | | Correctly handle infinity value in PostgreSQL range typeyuuji.yaginuma2018-01-043-2/+22
| |_|_|_|_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | An empty string is an invalid value in Ruby's range class. So need to handle `Float::INFINITY` as it is and cast it in `encode_range`. Fixes #31612
* | | | | | | Merge pull request #31331 from dinahshi/postgresql_bulk_updateMatthew Draper2018-01-034-13/+68
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | Add bulk alter support for PostgreSQL
| * | | | | | | Add bulk alter support for PostgreSQLDinah Shi2017-12-064-13/+68
| | | | | | | |
* | | | | | | | Merge pull request #25456 from ojab/masterRyuta Kamizono2018-01-032-4/+2
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove dormant check
| * | | | | | | | Remove dormant checkojab2016-06-211-3/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This check was introduced in 6edaa26 and moved through multiple refactorings. No test are broken after removal and AFAICS there is no way to trigger it.
* | | | | | | | | Merge pull request #27561 from fishbrain/count-all-in-has-many-associationRyuta Kamizono2018-01-034-1/+16
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use `count(:all)` in HasManyAssociation#count_records
| * | | | | | | | | Use `count(:all)` in HasManyAssociation#count_recordsKlas Eskilson2017-02-074-1/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem: Calling `count` on an association can cause invalid SQL queries to be created where the `SELECT COUNT(a, b, c)` function receives multiple columns. This will cause a `StatementInvalid` exception later on. Solution: Use `count(:all)`, which generates a `SELECT COUNT(*)...` query independently of the association. This also includes a test case that, before the fix, broke.
* | | | | | | | | | Merge pull request #29018 from willbryant/missing_attributes_after_saveRyuta Kamizono2018-01-031-0/+7
|\ \ \ \ \ \ \ \ \ \ | |_|_|_|_|_|_|_|_|/ |/| | | | | | | | | | | | | | | | | | | fix the dirty tracking code's save hook overwriting missing attribute…
generated by cgit v1.2.3 (git 2.25.1) at 2024-06-26 15:12:45 +0000