aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/lib
Commit message (Collapse)AuthorAgeFilesLines
* Make ActiveRecord `ConnectionPool.connections` thread-safe. (#36473)jeffdoering2019-06-131-1/+16
| | | | | | | | | | | | | | | | | | | | | | | * 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 #36472 from kamipo/empty_line_only_before_access_modifierRyuta Kamizono2019-06-13100-122/+0
|\ | | | | Enable `Layout/EmptyLinesAroundAccessModifier` cop
| * Enable `Layout/EmptyLinesAroundAccessModifier` copRyuta Kamizono2019-06-13100-122/+0
| | | | | | | | | | | | | | | | | | | | | | 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
* | Clear schema cache when a table is created/dropped/renamedRyuta Kamizono2019-06-134-0/+11
|/ | | | Otherwise `Model.table_exists?` returns the staled cache result.
* Merge pull request #36440 from malept/multi-db-abort_if_pending_migrations-taskEileen M. Uchitelle2019-06-111-1/+25
|\ | | | | 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-101-1/+25
| |
* | Allow column name with function (e.g. `length(title)`) as safe SQL stringRyuta Kamizono2019-06-104-8/+32
|/ | | | | | | | | | | | | | | | 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.
* Allow `column_name AS alias` as safe SQL stringRyuta Kamizono2019-06-104-0/+4
|
* Refactor `disallow_raw_sql!` to avoid `split(/\s*,\s*/)` to order argsRyuta Kamizono2019-06-096-21/+56
| | | | | `split(/\s*,\s*/)` to order args and then `permit.match?` one by one is much slower than `permit.match?` once.
* Add forgotten nodoc to dump_schema method.Wojciech Wnętrzak2019-06-071-1/+1
| | | | Method added in https://github.com/rails/rails/pull/36416
* Merge pull request #36429 from bogdan/fix-preloading-duplicate-recordsRyuta Kamizono2019-06-071-1/+3
|\ | | | | Fix preloading on AR::Relation where records are duplicated by a join
| * Fix preloading on AR::Relation where records are duplicated by a joinBogdan Gusiev2019-06-061-1/+3
| |
* | Merge pull request #36420 from kamipo/quoted_identifier_regexRyuta Kamizono2019-06-067-61/+137
|\ \ | | | | | | Allow quoted identifier string as safe SQL string
| * | Allow quoted identifier string as safe SQL stringRyuta Kamizono2019-06-067-61/+137
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently `posts.title` is regarded as a safe SQL string, but `"posts"."title"` (it is a result of `quote_table_name("posts.title")`) is regarded as an unsafe SQL string even though a result of `quote_table_name` should obviously be regarded as a safe SQL string, since the column name matcher doesn't respect quotation, it is a little annoying. This changes the column name matcher to allow quoted identifiers as safe SQL string, now all results of the `quote_table_name` are regarded as safe SQL string.
* | | Move schema cache from connection to pooleileencodes2019-06-055-6/+33
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR proposes moving the schema cache from the connection to the pool so the connection can ask the pool for the cache. In a future PR our goal is to be able to read the yaml file from the pool so we can get rid of the `active_record.check_schema_cache_dump` initializer. This will fix the issues surrounding dumping the schema cache and mulitple databases. Why do we want to get rid of the initializer you ask? Well I was looking at #34449 and trying to make it work for our usecase and it revealed A LOT of problems. There are a few issues that I will fix in remaining PRs with SchemaMigration, but there's a big glaring issue with this initializer. When you have an application with multiple databases we'll need to loop through all the configurations and set the schema cache on those connections. The problem is on initialization we only have one connection - the one for Ar::Base. This is fine in a single db application but not fine in multi-db. If we follow the pattern in #34449 and establish a connection to those other dbs we will end up setting the cache on the _connection object_ rather than on all connections that connect for that config. So even though we looped through the configs and assigned the cache the cache will not be set (or will be set wrong) once the app is booted because the connection objects after boot are _different_ than the connection objects we assigned the cache to. After trying many different ways to set the schema cache `@tenderlove` and I came to the conclusion that the initializer is problematic, as is setting the schema cache twice. This is part 1 to move the cache to the pool so the cache can read from the schema cache yaml file instead of setting it when initializing the app. To do this we have created a `NullPool` that initializes an empty cache. I put the `get_schema_cache` and `set_schema_cache` in an `AbstractPool` so we can share code between `ConnectionPool` and `NullPool` instead of duplicating code. Now we only need to set the schema_cache on the pool rather than the connection. In `discard!` we need to unset the connection from the schema_cache - we still want the cache just not the connection.
* | Merge pull request #36416 from freeletics/fix-db-prepareEileen M. Uchitelle2019-06-052-19/+49
|\ \ | | | | | | Fixed db:prepare task for multiple databases.
| * | Fixed db:prepare task for multiple databases.Wojciech Wnętrzak2019-06-052-19/+49
| | | | | | | | | | | | | | | When one database existed already, but not the other, during setup of missing one, existing database was wiped out.
* | | Merge pull request #36394 from eileencodes/treat-application-record-as-primaryEileen M. Uchitelle2019-06-051-2/+6
|\ \ \ | |/ / |/| | Treat ActiveRecord::Base and ApplicationRecord as "primary"
| * | Treat ActiveRecord::Base and ApplicationRecord as "primary"eileencodes2019-06-051-2/+6
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When someone has a multi-db application their `ApplicationRecord` will look like: ```ruby class ApplicationRecord < ActiveRecord::Base self.abstract_class = true connects_to database: { writing: :primary, reading: :replica } end ``` This will cause us to open 2 connections to ActiveRecord::Base's database when we actually only want 1. This is because Rails sees `ApplicationRecord` and thinks it's a new connection, not the existing `ActiveRecord::Base` connection because the `connection_specification_name` is different. This PR changes `ApplicationRecord` classes to consider themselves the same as the "primary" connection. Fixes #36382
* / Fix sqlite3 collation parsing when using decimal columns.Martin Schuster2019-06-041-2/+2
|/ | | | | | If an sqlite3 table contains a decimal column behind columns with a collation definition, then parsing the collation of all preceeding columns will fail -- the collation will be missed without notice.
* Merge pull request #36384 from ↵Ryuta Kamizono2019-06-031-1/+1
|\ | | | | | | | | guigs/fix-invalid-schema-when-pk-column-has-comment Fix invalid schema dump when primary key column has a comment
| * Fix invalid schema dump when primary key column has a commentGuilherme Goettems Schneider2019-06-031-1/+1
| | | | | | | | | | | | | | | | Before this fix it would either generate an invalid schema, passing `comment` option twice to `create_table`, or it move the comment from primary key column to the table if table had no comment when the dump was generated. The situation now is that a comment on primary key will be ignored (not present on schema). Fixes #29966
* | Refactor `create_table`'s options separationRyuta Kamizono2019-06-032-12/+11
| | | | | | | | | | | | | | | | `create_table` and `t.column` have the same named options (e.g. `:comment`, `:primary_key`), so it should be separated table options from column options. Related #36373.
* | Avoid making extra 5 arrays in each `save`Ryuta Kamizono2019-06-011-16/+26
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Each `save` calls `all_timestamp_attributes_in_model` to fill timestamp columns. Allthough the `all_timestamp_attributes_in_model` returns the same value every time, the `all_timestamp_attributes_in_model` makes extra 5 arrays every time. This avoids the making extra 5 arrays by memoizing the result, it makes `save` economical and a bit faster. https://gist.github.com/kamipo/1ddad2235073f508637bf9a72d64bb83 Before (2a015f6c0be0593a624b0c800e5335319ac4c660): ``` {["~/rails/activerecord/lib/active_record/timestamp.rb", 76, :T_ARRAY]=>[1000, 0, 341, 0, 1, 13640], ["~/rails/activerecord/lib/active_record/timestamp.rb", 64, :T_ARRAY]=>[1000, 0, 341, 0, 1, 13640], ["~/rails/activerecord/lib/active_record/timestamp.rb", 80, :T_ARRAY]=>[1000, 0, 341, 0, 1, 13640], ["~/rails/activerecord/lib/active_record/timestamp.rb", 68, :T_ARRAY]=>[1000, 0, 341, 0, 1, 13640], ["~/rails/activerecord/lib/active_record/timestamp.rb", 73, :T_ARRAY]=>[1000, 0, 341, 0, 1, 13640]} Warming up -------------------------------------- User.create * 10 36.000 i/100ms Calculating ------------------------------------- User.create * 10 353.644 (± 7.4%) i/s - 1.764k in 5.021876s ``` After (this change): ``` {["~/rails/activerecord/lib/active_record/timestamp.rb", 83, :T_ARRAY]=>[1, 0, 1, 1, 1, 40], ["~/rails/activerecord/lib/active_record/timestamp.rb", 87, :T_ARRAY]=>[1, 0, 1, 1, 1, 40], ["~/rails/activerecord/lib/active_record/timestamp.rb", 64, :T_ARRAY]=>[1, 1, 1, 1, 1, 0], ["~/rails/activerecord/lib/active_record/timestamp.rb", 69, :T_ARRAY]=>[1, 1, 1, 1, 1, 0], ["~/rails/activerecord/lib/active_record/timestamp.rb", 74, :T_ARRAY]=>[1, 1, 1, 1, 1, 0]} Warming up -------------------------------------- User.create * 10 37.000 i/100ms Calculating ------------------------------------- User.create * 10 380.063 (± 7.1%) i/s - 1.924k in 5.097917s ```
* Fix table comment also being applied to the primary key columnGuilherme Goettems Schneider2019-05-311-1/+1
|
* Remove wrong default value for `cache_versioning` in documentation of ↵Sebastian Röder2019-05-291-1/+1
| | | | | `cache_version` `ActiveRecord::Base.cache_versioning` it `true` by default since Rails 5.2 as stated correctly in the documentation for the `ActiveRecord::Base.cache_versioning` class attribute. Remove the wrong and duplicated documentation of the default value for `cache_versioning` from `cache_version`.
* `:datetime` and `:time` columns allows `:precision` option [ci skip]Ryuta Kamizono2019-05-281-2/+3
|
* Merge pull request #36350 from kamipo/fast_pluckRyuta Kamizono2019-05-281-3/+5
|\ | | | | Allow symbol (i.e. quoted identifier) as safe SQL string
| * Allow symbol (i.e. quoted identifier) as safe SQL stringRyuta Kamizono2019-05-281-3/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `pluck(:id)` / `order(:id)` are very common use case, and passed symbol (i.e. quoted identifier) is obviously safe argument, but `:id.to_s.split(/\s*,\s*/).all? { |part| permit.match?(part) }` is useless and a bit expensive operation for each such safe symbols (will make extra 2 mutable strings, 1 array, 1 proc). This avoids the expensive operation to such safe symbols, it makes `pluck(:id)` / `order(:id)` itself about 9% faster. https://gist.github.com/kamipo/11d428b57f3629a72ae89c6f21721326 Before (93e640735e9363672b770b8d1c5a35f9e464f806): ``` Warming up -------------------------------------- users.pluck(:id) 1.217k i/100ms users.order(:id).to_sql 1.848k i/100ms Calculating ------------------------------------- users.pluck(:id) 12.239k (± 8.2%) i/s - 60.850k in 5.013839s users.order(:id).to_sql 19.111k (± 7.5%) i/s - 96.096k in 5.064450s ``` After (this change): ``` Warming up -------------------------------------- users.pluck(:id) 1.293k i/100ms users.order(:id).to_sql 2.036k i/100ms Calculating ------------------------------------- users.pluck(:id) 13.257k (± 6.9%) i/s - 65.943k in 5.002568s users.order(:id).to_sql 20.957k (± 7.6%) i/s - 105.872k in 5.086102s ```
* | Use WeakRef to avoid leaking connection poolsJohn Hawthorn2019-05-271-11/+21
|/ | | | | | | | | | | | | | | | | | Prior to 3e2e8eeb9ea552bd4782538cf9348455f3d0e14a the Reaper thread would hold a reference to connection pools indefinitely, preventing the connection pool from being garbage collected, and also leaking the Thread. Since 3e2e8eeb9ea552bd4782538cf9348455f3d0e14a, there is only one Reaper Thread for all pools, however all pools are still stored in a class variable, preventing them from being garbage collected. This commit instead holds reference to the pools through a WeakRef. This way, connection pools referenced elsewhere will be reaped, any others will be able to be garbage collected. I don't love resorting to WeakRef to solve this, but I believe it's the simplest way to accomplish the the desired behaviour.
* Extract `readonly_attribute?`Ryuta Kamizono2019-05-273-6/+6
|
* Merge pull request #36320 from XrXr/no-doc-prependRafael França2019-05-211-1/+1
|\ | | | | [CI skip] Put :nodoc: on method that raises NoMethodError
| * Put :nodoc: on method that raises NoMethodErrorAlan Wu2019-05-211-1/+1
| | | | | | | | | | This method is not intended to be used so I think we should remove it from the docs.
* | Put all `explain` methods into `DatabaseStatements` moduleRyuta Kamizono2019-05-224-18/+14
| | | | | | | | | | Almost all database statements methods except `explain` was moved into `DatabaseStatements` at #35922. This moves the last one method.
* | Fall back to type casting from the connection adapterRyuta Kamizono2019-05-212-20/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Unfortunately, a11a8ff had no effect as long as using bind param, and was not tested. This ensures making the intent of a11a8ff, which fall back to type casting from the connection adapter. Fixes #35205. ``` % ARCONN=postgresql bundle exec ruby -w -Itest test/cases/relation/where_test.rb -n test_type_casting_nested_joins Using postgresql Run options: -n test_type_casting_nested_joins --seed 55730 # Running: E Error: ActiveRecord::WhereTest#test_type_casting_nested_joins: ActiveRecord::StatementInvalid: PG::InvalidTextRepresentation: ERROR: invalid input syntax for integer: "2-foo" rails test test/cases/relation/where_test.rb:30 Finished in 0.245778s, 4.0687 runs/s, 0.0000 assertions/s. 1 runs, 0 assertions, 0 failures, 1 errors, 0 skips ```
* | Implicit through table joins should be appeared before user supplied joinsRyuta Kamizono2019-05-191-17/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | #36293 was an issue for through association with `joins` for a long time, but since #35864 through association with `left_joins` would also be affected by the issue. Implicit through table joins should be appeared before user supplied joins, otherwise loading through association with joins will cause a statement invalid error. Fixes #36293. ``` % ARCONN=postgresql bundle exec ruby -w -Itest test/cases/associations/has_many_through_associations_test .rb -n test_through_association_with_joins Using postgresql Run options: -n test_through_association_with_joins --seed 7116 # Running: E Error: HasManyThroughAssociationsTest#test_through_association_with_joins: ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: missing FROM-clause entry for table "posts" LINE 1: ... "comments_posts" ON "comments_posts"."post_id" = "posts"."i... ^ : SELECT "comments".* FROM "comments" INNER JOIN "comments" "comments_posts" ON "comments_posts"."post_id" = "posts"."id" INNER JOIN "posts" ON "comments"."post_id" = "posts"."id" WHERE "posts"."author_id" = $1 rails test test/cases/associations/has_many_through_associations_test.rb:61 Finished in 0.388657s, 2.5730 runs/s, 0.0000 assertions/s. 1 runs, 0 assertions, 0 failures, 1 errors, 0 skips ```
* | Merge pull request #36296 from jhawthorn/dont_fear_the_reaperAaron Patterson2019-05-181-7/+25
|\ \ | | | | | | Use a single thread for all ConnectionPool Reapers
| * | Use a single thread for all ConnectionPool ReapersJohn Hawthorn2019-05-171-7/+25
| | | | | | | | | | | | | | | Previously we would spawn one thread per connection pool, which ends up being wasteful for apps with several connection pools.
* | | Merge pull request #36284 from kamipo/fix_eager_loading_with_string_joinsRyuta Kamizono2019-05-151-2/+11
|\ \ \ | | | | | | | | Fix eager loading associations with string joins not to raise NoMethodError
| * | | Fix eager loading associations with string joins not to raise NoMethodErrorRyuta Kamizono2019-05-151-2/+11
| | | | | | | | | | | | | | | | Fixes #34456.
* | | | Don't track implicit `touch` mutationRyuta Kamizono2019-05-132-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This partly reverts the effect of d1107f4d. d1107f4d makes `touch` tracks the mutation whether the `touch` is occurred by explicit or not. Existing apps expects that the previous changes tracks only the changes which is explicit action by users. I'd revert the implicit `touch` mutation tracking since I'd not like to break existing apps. Fixes #36219.
* | | | Merge pull request #36255 from eugeneius/sqlite_savepoint_supportKasper Timm Hansen2019-05-124-7/+5
|\ \ \ \ | | | | | | | | | | Remove SQLite version support caveats [ci skip]
| * | | | Remove SQLite version support caveats [ci skip]Eugene Kenny2019-05-124-7/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since d1a74c1e012ed96f7179e53b9190b7da0a369e11, Active Record requires SQLite version 3.8.0 or greater, so savepoints and partial indexes are always available. That commit also added a runtime version check, so we can remove the minimum version requirement from the internal adapter documentation.
* | | | | Merge pull request #36073 from itsWill/document_update_counters_on_relationRyuta Kamizono2019-05-121-1/+13
|\ \ \ \ \ | |/ / / / |/| | | | document update_counters on relation [ci skip]
| * | | | document update_counters on relation [ci skip]Guilherme Mansur2019-05-111-1/+13
| | | | |
* | | | | Merge branch 'migration-timestamp-fix'Gannon McGibbon2019-05-101-0/+1
|\ \ \ \ \ | |_|_|/ / |/| | | | | | | | | Closes #28707.
| * | | | Add class option timestamps to migration generatorMichael Duchemin2019-02-091-0/+1
| | | | | | | | | | | | | | | | | | | | Fixes GH#28706. Now rails g migration create_users and rails g model User have the same behavior for timestamps since they implement the same migration template. The expected behavior is that this create table migration will create the table with timestamps unless you pass --no-timestamps or --skip-timestamps to the generator. The expected migration should match what you get when you use the model generator. Using the migration generator, which doesn't have a class_option for timestamps would cause them to not be added to the migration file. Now the migration behavior of the migration generator, create_table only, is aligned with the migration behavior of the model generator. Also modified relevant example of ActiveRecord Migrations Guide.
* | | | | Remove ignored_sql from SQLCounter by adding "TRANSACTION" to log nameYasuo Honda2019-05-084-12/+12
| |_|/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This commit adds "TRANSACTION" to savepoint and commit, rollback statements because none of savepoint statements were removed by #36153 since they are not "SCHEMA" statements. Although, only savepoint statements can be labeled as "TRANSACTION" I think all of transaction related method should add this label. Follow up #36153
* | | | Properly give defaults for DatabaseSelector optionsAkira Matsuda2019-05-071-3/+3
| | | | | | | | | | | | | | | | | | | | The initializer receives `nil` for these options when no cofigurations were given: https://github.com/rails/rails/blob/v6.0.0.rc1/activerecord/lib/active_record/railtie.rb#L91-L97
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-08 02:30:57 +0000