Remove `--no-comments` from Postgres structure dumps

Reverts https://github.com/rails/rails/pull/43216
Fixes https://github.com/rails/rails/issues/44498

Per the discussion at https://github.com/rails/rails/pull/44603#discussion_r820875837, reverting the feature as it's not clear it should be the default. Users who don't want comments in structure dumps can use `ActiveRecord::Tasks::DatabaseTasks.structure_dump_flags = ['--no-comment']`.
This commit is contained in:
Alex Ghiculescu 2022-03-07 12:40:12 -06:00
parent 54c7357769
commit c94ac46cd8
5 changed files with 35 additions and 12 deletions

View File

@ -1,3 +1,14 @@
* Remove `--no-comments` flag in structure dumps for PostgreSQL
This broke some apps that used custom schema comments. If you don't want
comments in your structure dump, you can use:
```ruby
ActiveRecord::Tasks::DatabaseTasks.structure_dump_flags = ['--no-comments']
```
*Alex Ghiculescu*
* Reduce the memory footprint of fixtures accessors.
Until now fixtures accessors were eagerly defined using `define_method`.

View File

@ -58,7 +58,6 @@ module ActiveRecord
end
args = ["--schema-only", "--no-privileges", "--no-owner"]
args << "--no-comments" if connection.database_version >= 110_000
args.concat(["--file", filename])
args.concat(Array(extra_flags)) if extra_flags

View File

@ -375,7 +375,7 @@ if current_adapter?(:PostgreSQLAdapter)
def test_structure_dump_with_env
expected_env = { "PGHOST" => "my.server.tld", "PGPORT" => "2345", "PGUSER" => "jane", "PGPASSWORD" => "s3cr3t" }
expected_command = [expected_env, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comments", "--file", @filename, "my-app-db"]
expected_command = [expected_env, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"]
assert_called_with(Kernel, :system, expected_command, returns: true) do
ActiveRecord::Tasks::DatabaseTasks.structure_dump(
@ -387,7 +387,7 @@ if current_adapter?(:PostgreSQLAdapter)
def test_structure_dump_with_ssl_env
expected_env = { "PGSSLMODE" => "verify-full", "PGSSLCERT" => "client.crt", "PGSSLKEY" => "client.key", "PGSSLROOTCERT" => "root.crt" }
expected_command = [expected_env, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comments", "--file", @filename, "my-app-db"]
expected_command = [expected_env, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"]
assert_called_with(Kernel, :system, expected_command, returns: true) do
ActiveRecord::Tasks::DatabaseTasks.structure_dump(
@ -398,7 +398,7 @@ if current_adapter?(:PostgreSQLAdapter)
end
def test_structure_dump_with_extra_flags
expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comments", "--file", @filename, "--noop", "my-app-db"]
expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--noop", "my-app-db"]
assert_called_with(Kernel, :system, expected_command, returns: true) do
with_structure_dump_flags(["--noop"]) do
@ -408,7 +408,7 @@ if current_adapter?(:PostgreSQLAdapter)
end
def test_structure_dump_with_hash_extra_flags_for_a_different_driver
expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comments", "--file", @filename, "my-app-db"]
expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"]
assert_called_with(Kernel, :system, expected_command, returns: true) do
with_structure_dump_flags({ mysql2: ["--noop"] }) do
@ -418,7 +418,7 @@ if current_adapter?(:PostgreSQLAdapter)
end
def test_structure_dump_with_hash_extra_flags_for_the_correct_driver
expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comments", "--file", @filename, "--noop", "my-app-db"]
expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--noop", "my-app-db"]
assert_called_with(Kernel, :system, expected_command, returns: true) do
with_structure_dump_flags({ postgresql: ["--noop"] }) do
@ -436,7 +436,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comments", "--file", @filename, "-T", "foo", "-T", "bar", "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "-T", "foo", "-T", "bar", "my-app-db"],
returns: true
) do
ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename)
@ -450,7 +450,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comments", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"],
returns: true
) do
ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename)
@ -463,7 +463,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comments", "--file", @filename, "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"],
returns: true
) do
with_dump_schemas(:all) do
@ -476,7 +476,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comments", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"],
returns: true
) do
with_dump_schemas("foo,bar") do
@ -490,7 +490,7 @@ if current_adapter?(:PostgreSQLAdapter)
assert_called_with(
Kernel,
:system,
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comments", "--file", filename, "my-app-db"],
[{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", filename, "my-app-db"],
returns: nil
) do
e = assert_raise(RuntimeError) do

View File

@ -639,3 +639,16 @@ irb> Article.count
NOTE: This application only cares about non-archived `Articles`. A view also
allows for conditions so we can exclude the archived `Articles` directly.
Structure dumps
--------------
If your `config.active_record.schema_format` is `:sql`, Rails will call `pg_dump` to generate a
structure dump.
You can use `ActiveRecord::Tasks::DatabaseTasks.structure_dump_flags` to configure `pg_dump`.
For example, to exclude comments from your structure dump, add this to an initializer:
```ruby
ActiveRecord::Tasks::DatabaseTasks.structure_dump_flags = ['--no-comments']
```

View File

@ -976,7 +976,7 @@ should be large enough to accommodate both the foreground threads (.e.g web serv
Controls whether the Active Record MySQL adapter will consider all `tinyint(1)` columns as booleans. Defaults to `true`.
#### `ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.create_unlogged_table`
#### `ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.create_unlogged_tables`
Controls whether database tables created by PostgreSQL should be "unlogged", which can speed
up performance but adds a risk of data loss if the database crashes. It is