| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Every database executes different type of sql statement to get metadata then `ActiveRecord::TestCase` ignores these database specific sql statements to make `assert_queries` or `assert_no_queries` work consistently.
Connection adapter already labels these statement by setting "SCHEMA" argument, this pull request makes use of "SCHEMA" argument to ignore metadata queries.
Here are the details of these changes:
* PostgresqlConnectionTest
Each of PostgresqlConnectionTest modified just executes corresponding methods
https://github.com/rails/rails/blob/fef174f5c524edacbcad846d68400e7fe114a15a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb#L182-L195
```ruby
# Returns the current database encoding format.
def encoding
query_value("SELECT pg_encoding_to_char(encoding) FROM pg_database WHERE datname = current_database()", "SCHEMA")
end
# Returns the current database collation.
def collation
query_value("SELECT datcollate FROM pg_database WHERE datname = current_database()", "SCHEMA")
end
# Returns the current database ctype.
def ctype
query_value("SELECT datctype FROM pg_database WHERE datname = current_database()", "SCHEMA")
end
```
* BulkAlterTableMigrationsTest
mysql2 adapter executes `SHOW KEYS FROM ...` to see if there is an index already created as below. I think the main concerns of these tests are how each database adapter creates or drops indexes then ignoring `SHOW KEYS FROM` statement makes sense.
https://github.com/rails/rails/blob/fef174f5c524edacbcad846d68400e7fe114a15a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb#L11
```ruby
execute_and_free("SHOW KEYS FROM #{quote_table_name(table_name)}", "SCHEMA") do |result|
```
* Temporary change not included in this commit to show which statements executed
```diff
$ git diff
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index 8e8ed494d9..df05f9bd16 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -854,7 +854,7 @@ def test_adding_indexes
classname = ActiveRecord::Base.connection.class.name[/[^:]*$/]
expected_query_count = {
- "Mysql2Adapter" => 3, # Adding an index fires a query every time to check if an index already exists or not
+ "Mysql2Adapter" => 1, # Adding an index fires a query every time to check if an index already exists or not
"PostgreSQLAdapter" => 2,
}.fetch(classname) {
raise "need an expected query count for #{classname}"
@@ -886,7 +886,7 @@ def test_removing_index
classname = ActiveRecord::Base.connection.class.name[/[^:]*$/]
expected_query_count = {
- "Mysql2Adapter" => 3, # Adding an index fires a query every time to check if an index already exists or not
+ "Mysql2Adapter" => 1, # Adding an index fires a query every time to check if an index already exists or not
"PostgreSQLAdapter" => 2,
}.fetch(classname) {
raise "need an expected query count for #{classname}"
$
```
* Executed these modified tests
```ruby
$ ARCONN=mysql2 bin/test test/cases/migration_test.rb -n /index/
Using mysql2
Run options: -n /index/ --seed 8462
F
Failure:
BulkAlterTableMigrationsTest#test_adding_indexes [/home/yahonda/git/rails/activerecord/test/cases/migration_test.rb:863]:
3 instead of 1 queries were executed.
Queries:
SHOW KEYS FROM `delete_me`
SHOW KEYS FROM `delete_me`
ALTER TABLE `delete_me` ADD UNIQUE INDEX `awesome_username_index` (`username`), ADD INDEX `index_delete_me_on_name_and_age` (`name`, `age`).
Expected: 1
Actual: 3
bin/test test/cases/migration_test.rb:848
F
Failure:
BulkAlterTableMigrationsTest#test_removing_index [/home/yahonda/git/rails/activerecord/test/cases/migration_test.rb:895]:
3 instead of 1 queries were executed.
Queries:
SHOW KEYS FROM `delete_me`
SHOW KEYS FROM `delete_me`
ALTER TABLE `delete_me` DROP INDEX `index_delete_me_on_name`, ADD UNIQUE INDEX `new_name_index` (`name`).
Expected: 1
Actual: 3
bin/test test/cases/migration_test.rb:879
..
Finished in 0.379245s, 10.5473 runs/s, 7.9105 assertions/s.
4 runs, 3 assertions, 2 failures, 0 errors, 0 skips
$
```
* ActiveRecord::ConnectionAdapters::Savepoints
Left `self.ignored_sql` to ignore savepoint related statements because these SQL statements are not related "SCHEMA"
```
self.ignored_sql = [/^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/]
```
https://github.com/rails/rails/blob/fef174f5c524edacbcad846d68400e7fe114a15a/activerecord/lib/active_record/connection_adapters/abstract/savepoints.rb#L10-L20
```ruby
def create_savepoint(name = current_savepoint_name)
execute("SAVEPOINT #{name}")
end
def exec_rollback_to_savepoint(name = current_savepoint_name)
execute("ROLLBACK TO SAVEPOINT #{name}")
end
def release_savepoint(name = current_savepoint_name)
execute("RELEASE SAVEPOINT #{name}")
end
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
options
When I've added new `:size` option in #35071, I've found that invalid
`:limit` and `:precision` raises `ActiveRecordError` unlike other
invalid options.
I think that is hard to distinguish argument errors and statement
invalid errors since the `StatementInvalid` is a subclass of the
`ActiveRecordError`.
https://github.com/rails/rails/blob/c9e4c848eeeb8999b778fa1ae52185ca5537fffe/activerecord/lib/active_record/errors.rb#L103
```ruby
begin
# execute any migration
rescue ActiveRecord::StatementInvalid
# statement invalid
rescue ActiveRecord::ActiveRecordError, ArgumentError
# `ActiveRecordError` except `StatementInvalid` is maybe an argument error
end
```
I'd say this is the inconsistency worth fixing.
Before:
```ruby
add_column :items, :attr1, :binary, size: 10 # => ArgumentError
add_column :items, :attr2, :decimal, scale: 10 # => ArgumentError
add_column :items, :attr3, :integer, limit: 10 # => ActiveRecordError
add_column :items, :attr4, :datetime, precision: 10 # => ActiveRecordError
```
After:
```ruby
add_column :items, :attr1, :binary, size: 10 # => ArgumentError
add_column :items, :attr2, :decimal, scale: 10 # => ArgumentError
add_column :items, :attr3, :integer, limit: 10 # => ArgumentError
add_column :items, :attr4, :datetime, precision: 10 # => ArgumentError
```
|
|
|
|
|
| |
Also, `reset_column_information` is unnecessary since `reset_table_name`
does that too.
|
|
|
|
|
|
|
| |
This test is only executed for OracleAdapter.
https://github.com/rsim/oracle-enhanced/pull/1846 adds
an equivalent spec for Oracle enhanced adapter.
|
|
|
|
|
| |
Also, improving an argument error message for `limit`, extracting around
`type_to_sql` code into schema statements, and more exercise tests.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In MySQL, the text column size is 65,535 bytes by default (1 GiB in
PostgreSQL). It is sometimes too short when people want to use a text
column, so they sometimes change the text size to mediumtext (16 MiB) or
longtext (4 GiB) by giving the `limit` option.
Unlike MySQL, PostgreSQL doesn't allow the `limit` option for a text
column (raises ERROR: type modifier is not allowed for type "text").
So `limit: 4294967295` (longtext) couldn't be used in Action Text.
I've allowed changing text and blob size without giving the `limit`
option, it prevents that migration failure on PostgreSQL.
|
| |
|
|
|
|
|
|
|
| |
`connection.assume_migrated_upto_version`
Since #31727, `migrations_paths` in `assume_migrated_upto_version` is no
longer used.
|
|
|
|
| |
This was introduced at https://github.com/rails/rails/commit/cfa1df4b07bee5b2bbcbf9edd2ac287b4fb23c18#diff-b36b9c41be30b05dc14d09d7f3b192efR436.
|
|
|
|
|
|
|
|
|
|
| |
Currently we sometimes find a redundant begin block in code review
(e.g. https://github.com/rails/rails/pull/33604#discussion_r209784205).
I'd like to enable `Style/RedundantBegin` cop to avoid that, since
rescue/else/ensure are allowed inside do/end blocks in Ruby 2.5
(https://bugs.ruby-lang.org/issues/12906), so we'd probably meets with
that situation than before.
|
|
|
|
| |
[fatkodima & Stefan Kanev]
|
| |
|
|
|
|
|
|
|
| |
block
`ActiveRecord::MigrationContext.new` just create an instance, doesn't
have any side-effect.
|
| |
|
|
|
|
| |
Follow up of #32605.
|
|
|
|
|
| |
This autocorrects the violations after adding a custom cop in
3305c78dcd.
|
|
|
|
|
|
| |
73e7aab behaved as expected on codeship, failing the build with
exactly these RuboCop violations. Hopefully `rubocop -a` will
have been enough to get a passing build!
|
|
|
|
| |
Related to https://github.com/rails/rails/commit/d4eb0dc89ee6b476e2e10869dc282a96f956c6c7#r27830891
|
|\
| |
| |
| |
| | |
tgxworld/raise_error_when_advisory_lock_is_not_releases
Raise an error if advisory lock in migrator was not released.
|
| | |
|
| | |
|
| | |
|
|\ \
| | |
| | | |
Postgresql bulk_change_table should flatten procs array
|
| | | |
|
|/ /
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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.
|
|/ |
|
| |
|
| |
|
|
|
|
| |
`initialize_internal_metadata_table`
|
| |
|
| |
|
|
|
|
|
| |
Since #29540, `# frozen_string_literal: true` included original
migration files.
|
|
|
|
|
| |
This reverts commit 3420a14590c0e6915d8b6c242887f74adb4120f9, reversing
changes made to afb66a5a598ce4ac74ad84b125a5abf046dcf5aa.
|
| |
|
|
|
|
|
| |
If a relation has binds, `connection.to_sql(relation)` without binds
will generate invalid SQL. It should use `relation.to_sql` in that case.
|
| |
|
|
|
|
|
| |
Since 67fba0cf `SchemaMigration` model was extracted.
Use `SchemaMigration.table_name` instead.
|
|
|
|
| |
Simply use `SchemaMigration.table_name` instead.
|
|
|
|
|
|
|
| |
`supports_migrations?` was added at 4160b518 to determine if schema
statements (`create_table`, `drop_table`, etc) are implemented in the
adapter. But all tested databases has been supported migrations since
a4fc93c3 at least.
|
|
|
|
|
|
| |
`ColumnDumper#migration_keys` was extracted to customize keys for
standardized column arguments widths. But the feature was removed in
df84e98. The internal method is no longer used for that.
|
|
|
|
|
|
| |
`initialize_internal_metadata_table`
These internal initialize methods are no longer used internally.
|
|
|
|
|
| |
This reverts commit 39c77eb1843f79925c7195e8869afc7cb7323682, reversing
changes made to 9f6f51be78f8807e18fc6562c57af2fdbf8ccb56.
|
|
|
|
|
|
|
|
|
|
|
|
| |
`initialize_internal_metadata_table` internal public methods
These internal methods accidentally appeared in the doc, and so almost
useless. It is enough to create these internal tables directly, and
indeed do so in several places.
https://github.com/rails/rails/blob/v5.0.1/activerecord/lib/active_record/schema.rb#L55
https://github.com/rails/rails/blob/v5.0.1/activerecord/lib/active_record/railties/databases.rake#L6
https://github.com/rails/rails/blob/v5.0.1/activerecord/lib/active_record/tasks/database_tasks.rb#L230
|
|
|
|
|
|
|
|
|
|
|
| |
- Reference: https://travis-ci.org/rails/rails/jobs/189764676
- Reproduction command:
MTB_VERBOSE=2 bundle exec minitest_bisect --seed 33328 -Itest "test/cases/migration_test.rb" "test/cases/tasks/database_tasks_test.rb"
- You need to also add minitest_bisect gem to the Gemfile to reproduce
this failure.
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
invalid column type
/activerecord/lib/active_record/connection_adapters
/abstract/schema_definitions.rb:306
type = type.to_sym
Changed to the following to handle nil case:
type = type.to_sym if type
Added regression test for this case:
/activerecord/test/cases/migration_test.rb:554
if current_adapter?(:SQLite3Adapter)
def test_allows_sqlite3_rollback_on_invalid_column_type
Person.connection.create_table :something, force: true do |t|
t.column :number, :integer
t.column :name, :string
t.column :foo, :bar
end
assert Person.connection.column_exists?(:something, :foo)
assert_nothing_raised { Person.connection.remove_column :something, :foo, :bar }
assert !Person.connection.column_exists?(:something, :foo)
assert Person.connection.column_exists?(:something, :name)
assert Person.connection.column_exists?(:something, :number)
ensure
Person.connection.drop_table :something, if_exists: true
end
end
|
|
|
|
|
|
|
|
| |
Style/SpaceBeforeBlockBraces
Style/SpaceInsideBlockBraces
Style/SpaceInsideHashLiteralBraces
Fix all violations in the repository.
|
|
|
|
|
|
|
|
|
| |
A few have been left for aesthetic reasons, but have made a pass
and removed most of them.
Note that if the method `foo` returns an array, `foo << 1`
is a regular push, nothing to do with assignments, so
no self required.
|