From d5745607463ef72adb6a34e3ecc9b3820484a33f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jannosch=20Mu=CC=88ller?= Date: Mon, 13 Sep 2021 16:48:55 +0200 Subject: [PATCH] Avoid comment calls in pg:dump Fixes #36816 Fixes #43107 --- activerecord/CHANGELOG.md | 9 ++++++++ .../tasks/postgresql_database_tasks.rb | 6 ++++- .../test/cases/tasks/postgresql_rake_test.rb | 22 +++++++++---------- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 3c3587261f..e26dea2f82 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Avoid COMMENT statements in PostgreSQL structure dumps + + COMMENT statements are now omitted from the output of `db:structure:dump` when using PostgreSQL >= 11. + This allows loading the dump without a pgsql superuser account. + + Fixes #36816, #43107. + + *Janosch Müller* + * Add ssl support for postgresql database tasks Add `PGSSLMODE`, `PGSSLCERT`, `PGSSLKEY` and `PGSSLROOTCERT` to pg_env from database config diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb index 5c7481c75e..3913ef938f 100644 --- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb @@ -57,8 +57,12 @@ module ActiveRecord ActiveRecord.dump_schemas end - args = ["--schema-only", "--no-privileges", "--no-owner", "--file", filename] + args = ["--schema-only", "--no-privileges", "--no-owner"] + args << "--no-comment" if connection.database_version >= 110_000 + args.concat(["--file", filename]) + args.concat(Array(extra_flags)) if extra_flags + unless search_path.blank? args += search_path.split(",").map do |part| "--schema=#{part.strip}" diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb index 24f84fe950..62c99099e3 100644 --- a/activerecord/test/cases/tasks/postgresql_rake_test.rb +++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb @@ -355,7 +355,7 @@ if current_adapter?(:PostgreSQLAdapter) assert_called_with( Kernel, :system, - [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"], + [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", @filename, "my-app-db"], returns: true ) do ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename) @@ -373,7 +373,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", "--file", @filename, "my-app-db"] + expected_command = [expected_env, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", @filename, "my-app-db"] assert_called_with(Kernel, :system, expected_command, returns: true) do ActiveRecord::Tasks::DatabaseTasks.structure_dump( @@ -385,7 +385,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", "--file", @filename, "my-app-db"] + expected_command = [expected_env, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", @filename, "my-app-db"] assert_called_with(Kernel, :system, expected_command, returns: true) do ActiveRecord::Tasks::DatabaseTasks.structure_dump( @@ -396,7 +396,7 @@ if current_adapter?(:PostgreSQLAdapter) end def test_structure_dump_with_extra_flags - expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--noop", "my-app-db"] + expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", @filename, "--noop", "my-app-db"] assert_called_with(Kernel, :system, expected_command, returns: true) do with_structure_dump_flags(["--noop"]) do @@ -406,7 +406,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", "--file", @filename, "my-app-db"] + expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", @filename, "my-app-db"] assert_called_with(Kernel, :system, expected_command, returns: true) do with_structure_dump_flags({ mysql2: ["--noop"] }) do @@ -416,7 +416,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", "--file", @filename, "--noop", "my-app-db"] + expected_command = [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", @filename, "--noop", "my-app-db"] assert_called_with(Kernel, :system, expected_command, returns: true) do with_structure_dump_flags({ postgresql: ["--noop"] }) do @@ -434,7 +434,7 @@ if current_adapter?(:PostgreSQLAdapter) assert_called_with( Kernel, :system, - [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "-T", "foo", "-T", "bar", "my-app-db"], + [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", @filename, "-T", "foo", "-T", "bar", "my-app-db"], returns: true ) do ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename) @@ -448,7 +448,7 @@ if current_adapter?(:PostgreSQLAdapter) assert_called_with( Kernel, :system, - [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"], + [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"], returns: true ) do ActiveRecord::Tasks::DatabaseTasks.structure_dump(@configuration, @filename) @@ -461,7 +461,7 @@ if current_adapter?(:PostgreSQLAdapter) assert_called_with( Kernel, :system, - [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "my-app-db"], + [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", @filename, "my-app-db"], returns: true ) do with_dump_schemas(:all) do @@ -474,7 +474,7 @@ if current_adapter?(:PostgreSQLAdapter) assert_called_with( Kernel, :system, - [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"], + [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", @filename, "--schema=foo", "--schema=bar", "my-app-db"], returns: true ) do with_dump_schemas("foo,bar") do @@ -488,7 +488,7 @@ if current_adapter?(:PostgreSQLAdapter) assert_called_with( Kernel, :system, - [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--file", filename, "my-app-db"], + [{}, "pg_dump", "--schema-only", "--no-privileges", "--no-owner", "--no-comment", "--file", filename, "my-app-db"], returns: nil ) do e = assert_raise(RuntimeError) do