aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/test
Commit message (Collapse)AuthorAgeFilesLines
* Merge pull request #36565 from rails/fix-url-configsEileen M. Uchitelle2019-06-272-1/+32
|\ | | | | Fix broken url configs
| * Fix broken url configseileencodes2019-06-272-1/+32
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This PR is to fix #36559 but I also found other issues that haven't been reported. The check for `(config.size == 1 && config.values.all? { |v| v.is_a? String })` was naive. The only reason this passed was because we had tests that had single hash size configs, but that doesn't mean we don't want to create a hash config in other cases. So this now checks for `config["database"] || config["adapter"] || ENV["DATABASE_URL"]`. In the end for url configs we still get a UrlConfig but we need to pass through the HashConfig to create the right kind of UrlConfig. The UrlConfig's are really complex and I don't necessarily understand everything that's needed in order to act the same as Rails 5.2. I edited the connection handler test to demonstrate how the previous implementation was broken when checking config size. Now old and new tests pass so I think this is closer to 5.2. Fixes #36559
* | Address to "DEPRECATION WARNING: Uniqueness validator will no longer enforce ↵Ryuta Kamizono2019-06-281-1/+1
|/ | | | | | case sensitive comparison in Rails 6.1" Caused by #36210.
* `length(title)` is a safe SQL string since #36448Ryuta Kamizono2019-06-261-2/+2
|
* Merge pull request #36210 from ↵Rafael França2019-06-243-0/+48
|\ | | | | | | | | vishaltelangre/raise-record-invalid-when-associations-fail-to-save-due-to-uniqueness-failure Fix: ActiveRecord::RecordInvalid is not raised when an associated record fails to #save! due to uniqueness validation failure
| * Fix: ActiveRecord::RecordInvalid is not raised when an associated record ↵Vishal Telangre2019-05-103-0/+48
| | | | | | | | | | | | | | | | fails to #save! due to uniqueness validation failure Add tests Fix tests failing due to introduction of uniquness rule added to Book model
* | Merge pull request #36526 from yahonda/test_statement_cache_with_in_clause_pgRyuta Kamizono2019-06-211-1/+1
|\ \ | | | | | | Address test_statement_cache_with_in_clause failure
| * | Address test_statement_cache_with_in_clause failure due to nondeterministic ↵Yasuo Honda2019-06-201-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | sort order This failure is occasional, does not always reproduce. ```ruby $ cd activerecord $ bundle exec rake test_postgresql ... snip ... ....F Failure: ActiveRecord::BindParameterTest#test_statement_cache_with_in_clause [/home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:97]: Expected: [1, 3] Actual: [3, 1] rails test home/yahonda/git/rails/activerecord/test/cases/bind_parameter_test.rb:93 ```
* | | Revert schema dumper to use strings rather than integerseileencodes2019-06-201-0/+1
|/ / | | | | | | | | | | | | | | | | | | | | I think we should change this, but not in 6-0-stable since that's already in RC and I was trying to only make changes that won't require any app changes. This reverts a portion of https://github.com/rails/rails/pull/36439 that made all schema migration version numbers get dumped as an integer. While it doesn't _really_ matter it did change behavior. We should bring this back in 6.1 with a deprecation.
* | Merge pull request #36520 from kamipo/test_case_for_deterministic_orderRyuta Kamizono2019-06-201-0/+6
|\ \ | | | | | | Add test cases to ensure deterministic order for ordinal methods
| * | Add test cases to ensure deterministic order for ordinal methodsRyuta Kamizono2019-06-191-0/+6
| | | | | | | | | | | | | | | | | | Before 1340498d2, `order` with no-op value (e.g. `nil`, `""`) had broken the contract of ordinal methods, which returns a result deterministic ordered.
* | | Better error message for calling columns_hashGuilherme Mansur2019-06-191-0/+8
|/ / | | | | | | | | | | | | | | | | | | | | | | When a record does not have a table name, as in the case for a record with `self.abstract_class = true` and no `self.table_name` set the error message raises a cryptic: "ActiveRecord::StatementInvalid: Could not find table ''" this patch now raises a new `TableNotSpecified Error` Fixes: #36274 Co-Authored-By: Eugene Kenny <elkenny@gmail.com>
* | Merge pull request #35891 from Shopify/schema-cache-deduplicationKasper Timm Hansen2019-06-194-14/+18
|\ \ | | | | | | Deduplicate various Active Record schema cache structures
| * | Deduplicate various Active Record schema cache structuresJean Boussier2019-06-034-14/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Real world database schemas contain a lot of duplicated data. Some column names like `id`, `created_at` etc can easily be repeated hundreds of times. Same for SqlTypeMetada, most database will contain only a limited number of possible combinations. This result in a lot of wasted memory. The idea here is to make these data sctructures immutable, use a registry to substitute similar instances with pre-existing ones.
* | | PostgreSQL: Fix GROUP BY with ORDER BY virtual count attributeRyuta Kamizono2019-06-173-2/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | GROUP BY with virtual count attribute is invalid for almost all databases, but it is valid for PostgreSQL, and it had worked until Rails 5.2.2, so it is a regression for Rails 5.2.3 (caused by 311f001). I can't find perfectly solution for fixing this for now, but I would not like to break existing apps, so I decided to allow referencing virtual count attribute in ORDER BY clause when GROUP BY aggrigation (it partly revert the effect of 311f001) to fix the regression #36022. Fixes #36022.
* | | Remove unused `Arel::Attributes.for`Ryuta Kamizono2019-06-151-41/+0
| | | | | | | | | | | | `Arel::Attributes.for` is no longer used since https://github.com/rails/arel/pull/196.
* | | 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
| | |
* | | Move while_preventing_writes from conn to handlereileencodes2019-06-145-32/+92
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | | Move SchemaMigration to migration_contexteileencodes2019-06-149-114/+355
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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]
* | | Fix rubocop violationsRyuta Kamizono2019-06-141-2/+2
| | |
* | | Make ActiveRecord `ConnectionPool.connections` thread-safe. (#36473)jeffdoering2019-06-131-0/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 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]
* | | 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-1351-56/+1
|\ \ \ | | | | | | | | Enable `Layout/EmptyLinesAroundAccessModifier` cop
| * | | Enable `Layout/EmptyLinesAroundAccessModifier` copRyuta Kamizono2019-06-1351-56/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
* | | | Clear schema cache when a table is created/dropped/renamedRyuta Kamizono2019-06-132-4/+7
|/ / / | | | | | | | | | Otherwise `Model.table_exists?` returns the staled cache result.
* | | Merge pull request #36448 from ↵Ryuta Kamizono2019-06-113-34/+35
|\ \ \ | | | | | | | | | | | | | | | | 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-103-34/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
|/ / /
* | | Allow `column_name AS alias` as safe SQL stringRyuta Kamizono2019-06-101-0/+10
| | |
* | | NULLS { FIRST | LAST } is safe SQL string since ↵Ryuta Kamizono2019-06-071-6/+6
| | | | | | | | | | | | | | | | | | 6c82b6c99d86f37e61f935fb342cccd725d6c7d4 There is no need to be wrapped by `Arel.sql()`.
* | | Merge pull request #36429 from bogdan/fix-preloading-duplicate-recordsRyuta Kamizono2019-06-071-0/+5
|\ \ \ | | | | | | | | 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-0/+5
| | | |
* | | | Merge pull request #36426 from abhaynikam/bump-codeclimate-rubocop-versionRyuta Kamizono2019-06-061-2/+0
|\ \ \ \ | | | | | | | | | | Bump rubocop to 0.71
| * | | | Bump rubocop to 0.71Abhay Nikam2019-06-061-2/+0
| | | | |
* | | | | Merge pull request #36420 from kamipo/quoted_identifier_regexRyuta Kamizono2019-06-063-7/+29
|\ \ \ \ \ | |/ / / / |/| | | | Allow quoted identifier string as safe SQL string
| * | | | Allow quoted identifier string as safe SQL stringRyuta Kamizono2019-06-063-7/+29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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-051-1/+0
|/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 #36394 from eileencodes/treat-application-record-as-primaryEileen M. Uchitelle2019-06-051-0/+13
|\ \ \ \ | | | | | | | | | | Treat ActiveRecord::Base and ApplicationRecord as "primary"
| * | | | Treat ActiveRecord::Base and ApplicationRecord as "primary"eileencodes2019-06-051-0/+13
| |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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-0/+9
|/ / / | | | | | | | | | | | | | | | 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/+21
|\ \ \ | |/ / |/| | | | | | | | 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/+21
| | | | | | | | | | | | | | | | | | | | | | | | 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
* | | Address test_pluck_columns_with_same_name failure due to nondeterministic ↵Yasuo Honda2019-06-021-1/+1
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | sort order ```ruby $ bundle exec rake test_postgresql ... snip ... Failure: CalculationsTest#test_pluck_columns_with_same_name [/home/yahonda/git/rails/activerecord/test/cases/calculations_test.rb:842]: --- expected +++ actual @@ -1 +1 @@ -[["The First Topic", "The Second Topic of the day"], ["The Third Topic of the day", "The Fourth Topic of the day"]] +[["The Third Topic of the day", "The Fourth Topic of the day"], ["The First Topic", "The Second Topic of the day"]] ```
* | Fix table comment also being applied to the primary key columnGuilherme Goettems Schneider2019-05-311-0/+5
| |
* | Address intermittent CI failure due to non-determined sort orderRyuta Kamizono2019-05-301-6/+7
| | | | | | | | https://buildkite.com/rails/rails/builds/61384#ad441461-87d8-4bdc-a71f-61921fe2df2e/993-1004
* | Address intermittent CI failure in cascaded_eager_loading_test.rbRyuta Kamizono2019-05-291-12/+12
| | | | | | | | https://buildkite.com/rails/rails/builds/61362#99165d42-172d-4ad5-bf72-b29d8cd44f3e/995-1006
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-09 11:05:53 +0000