diff --git a/README.md b/README.md index ab7d0002..bed2569a 100644 --- a/README.md +++ b/README.md @@ -78,6 +78,21 @@ Scenic detected that we already had an existing `search_results` view at version update to the version 2 schema. All that's left for you to do is tweak the schema in the new definition and run the `update_view` migration. +If your view has dependent objects (e.g. other views) you can pass `cascade: +true` to the `update_view` call to drop and recreate your view's children too: + +```ruby +class UpdateSearchResultsToVersion2 < ActiveRecord::Migration + def change + update_view :search_results, version: 2, revert_to_version: 1, cascade: true + end +end +``` + +If your view has any dependent _materialized_ views with indexes, those +indexes will be recreated too. If you have a complex heirarchy of materialized +views with expensive calculations and large indexes this could take some time. + ## What if I want to change a view without dropping it? The `update_view` statement used by default will drop your view then create @@ -201,6 +216,14 @@ def change end ``` +If your view has dependencies and you want Scenic to drop those too: + +```ruby +def change + drop_view :search_results, revert_to_version: 2, cascade: true +end +``` + ## FAQs **Why do I get an error when querying a view-backed model with `find`, `last`, or `first`?** diff --git a/lib/scenic/adapters/postgres.rb b/lib/scenic/adapters/postgres.rb index 4eb100a5..1208c765 100644 --- a/lib/scenic/adapters/postgres.rb +++ b/lib/scenic/adapters/postgres.rb @@ -75,11 +75,25 @@ def create_view(name, sql_definition) # # @param name The name of the view to update # @param sql_definition The SQL schema for the updated view. + # @param cascade Whether to drop and recreate dependent objects or not # # @return [void] - def update_view(name, sql_definition) - drop_view(name) + def update_view(name, sql_definition, cascade=false) + if cascade + # Get existing views that could be dependent on this one. + existing_views = views.reject{|v| v.name == name} + + # Get indexes of existing materialized views + indexes = Indexes.new(connection: connection) + view_indexes = existing_views.select(&:materialized).flat_map do |view| + indexes.on(view.name) + end + end + + drop_view(name, cascade) create_view(name, sql_definition) + + recreate_dropped_views(existing_views, views, view_indexes) if cascade end # Replaces a view in the database using `CREATE OR REPLACE VIEW`. @@ -112,10 +126,11 @@ def replace_view(name, sql_definition) # This is typically called in a migration via {Statements#drop_view}. # # @param name The name of the view to drop + # @param cascade Whether to drop dependent objects or not # # @return [void] - def drop_view(name) - execute "DROP VIEW #{quote_table_name(name)};" + def drop_view(name, cascade=false) + execute "DROP VIEW #{quote_table_name(name)}#{" CASCADE" if cascade};" end # Creates a materialized view in the database @@ -144,18 +159,32 @@ def create_materialized_view(name, sql_definition) # # @param name The name of the view to update # @param sql_definition The SQL schema for the updated view. + # @param cascade Whether to drop and recreate dependent objects or not # # @raise [MaterializedViewsNotSupportedError] if the version of Postgres # in use does not support materialized views. # # @return [void] - def update_materialized_view(name, sql_definition) + def update_materialized_view(name, sql_definition, cascade=false) raise_unless_materialized_views_supported + if cascade + # Get existing views that could be dependent on this one. + existing_views = views.reject{|v| v.name == name} + + # Get indexes of existing materialized views + indexes = Indexes.new(connection: connection) + view_indexes = existing_views.select(&:materialized).flat_map do |view| + indexes.on(view.name) + end + end + IndexReapplication.new(connection: connection).on(name) do - drop_materialized_view(name) + drop_materialized_view(name, cascade) create_materialized_view(name, sql_definition) end + + recreate_dropped_views(existing_views, views, view_indexes) if cascade end # Drops a materialized view in the database @@ -163,13 +192,14 @@ def update_materialized_view(name, sql_definition) # This is typically called in a migration via {Statements#update_view}. # # @param name The name of the materialized view to drop. + # @param cascade Whether to drop dependent objects or not. # @raise [MaterializedViewsNotSupportedError] if the version of Postgres # in use does not support materialized views. # # @return [void] - def drop_materialized_view(name) + def drop_materialized_view(name, cascade=false) raise_unless_materialized_views_supported - execute "DROP MATERIALIZED VIEW #{quote_table_name(name)};" + execute "DROP MATERIALIZED VIEW #{quote_table_name(name)}#{" CASCADE" if cascade};" end # Refreshes a materialized view from its SQL schema. @@ -238,6 +268,24 @@ def refresh_dependencies_for(name) connection, ) end + + def recreate_dropped_views(old_views, current_views, indexes=[]) + index_reapplier = IndexReapplication.new(connection: connection) + + # Find any views that were lost + dropped_views = old_views.reject{|ov| current_views.any?{|cv| ov.name == cv.name}} + # Recreate them + dropped_views.each do |view| + if view.materialized + create_materialized_view view.name, view.definition + # Also recreate any indexes that were lost + lost_indexes = indexes.select{|index| index.object_name == view.name} + lost_indexes.each{|index| index_reapplier.try_index_create index} + else + create_view view.name, view.definition + end + end + end end end end diff --git a/lib/scenic/adapters/postgres/index_reapplication.rb b/lib/scenic/adapters/postgres/index_reapplication.rb index 302a1d55..b7cb9777 100644 --- a/lib/scenic/adapters/postgres/index_reapplication.rb +++ b/lib/scenic/adapters/postgres/index_reapplication.rb @@ -35,10 +35,6 @@ def on(name) indexes.each(&method(:try_index_create)) end - private - - attr_reader :connection, :speaker - def try_index_create(index) success = with_savepoint(index.index_name) do connection.execute(index.definition) @@ -51,6 +47,10 @@ def try_index_create(index) end end + private + + attr_reader :connection, :speaker + def with_savepoint(name) connection.execute("SAVEPOINT #{name}") yield diff --git a/lib/scenic/command_recorder.rb b/lib/scenic/command_recorder.rb index 74a2a311..124b2ce8 100644 --- a/lib/scenic/command_recorder.rb +++ b/lib/scenic/command_recorder.rb @@ -45,6 +45,11 @@ def perform_scenic_inversion(method, args) raise ActiveRecord::IrreversibleMigration, message end + if method == :create_view && scenic_args.cascade + message = "#{method} is not reversible if dependent objects were also dropped with CASCADE" + raise ActiveRecord::IrreversibleMigration, message + end + [method, scenic_args.invert_version.to_a] end end diff --git a/lib/scenic/command_recorder/statement_arguments.rb b/lib/scenic/command_recorder/statement_arguments.rb index 07a028bb..01e654c1 100644 --- a/lib/scenic/command_recorder/statement_arguments.rb +++ b/lib/scenic/command_recorder/statement_arguments.rb @@ -18,6 +18,10 @@ def revert_to_version options[:revert_to_version] end + def cascade + options[:cascade] + end + def invert_version StatementArguments.new([view, options_for_revert]) end diff --git a/lib/scenic/index.rb b/lib/scenic/index.rb index 0e48745f..699c70f9 100644 --- a/lib/scenic/index.rb +++ b/lib/scenic/index.rb @@ -32,5 +32,11 @@ def initialize(object_name:, index_name:, definition:) @index_name = index_name @definition = definition end + + def ==(index) + @object_name == index.object_name && + @index_name = index.index_name && + @definition == index.definition + end end end diff --git a/lib/scenic/statements.rb b/lib/scenic/statements.rb index 504453e7..4a2504ff 100644 --- a/lib/scenic/statements.rb +++ b/lib/scenic/statements.rb @@ -50,16 +50,18 @@ def create_view(name, version: nil, sql_definition: nil, materialized: false) # `version` argument to {#create_view}. # @param materialized [Boolean] Set to true if dropping a meterialized view. # defaults to false. + # @param cascade [Boolean] Set to true if dependent objects should also be + # dropped. defaults to false. # @return The database response from executing the drop statement. # # @example Drop a view, rolling back to version 3 on rollback # drop_view(:users_who_recently_logged_in, revert_to_version: 3) # - def drop_view(name, revert_to_version: nil, materialized: false) + def drop_view(name, revert_to_version: nil, materialized: false, cascade: false) if materialized - Scenic.database.drop_materialized_view(name) + Scenic.database.drop_materialized_view(name, cascade) else - Scenic.database.drop_view(name) + Scenic.database.drop_view(name, cascade) end end @@ -77,12 +79,14 @@ def drop_view(name, revert_to_version: nil, materialized: false) # `rake db rollback` # @param materialized [Boolean] True if updating a materialized view. # Defaults to false. + # @param cascade [Boolean] Set to true if dependent objects should also be + # dropped. defaults to false. # @return The database response from executing the create statement. # # @example # update_view :engagement_reports, version: 3, revert_to_version: 2 # - def update_view(name, version: nil, sql_definition: nil, revert_to_version: nil, materialized: false) + def update_view(name, version: nil, sql_definition: nil, revert_to_version: nil, materialized: false, cascade: false) if version.blank? && sql_definition.blank? raise( ArgumentError, @@ -100,9 +104,9 @@ def update_view(name, version: nil, sql_definition: nil, revert_to_version: nil, sql_definition ||= definition(name, version) if materialized - Scenic.database.update_materialized_view(name, sql_definition) + Scenic.database.update_materialized_view(name, sql_definition, cascade) else - Scenic.database.update_view(name, sql_definition) + Scenic.database.update_view(name, sql_definition, cascade) end end diff --git a/spec/integration/cascade_spec.rb b/spec/integration/cascade_spec.rb new file mode 100644 index 00000000..c4b18aee --- /dev/null +++ b/spec/integration/cascade_spec.rb @@ -0,0 +1,120 @@ +require "spec_helper" + +RSpec.shared_examples "a cascading migration" do |materialized| + it 'recreates the dependent view' do + views = Scenic::Adapters::Postgres::Views.new(connection) + run_migration(migration_for_create(materialized: materialized), :up) + expect { + run_migration(migration_for_update(materialized: materialized), :up) + }.to_not change { + views.all.length + } + end + + it 'recreates indexes on the dependent view' do + indexes = Scenic::Adapters::Postgres::Indexes.new(connection: connection) + run_migration(migration_for_create_materialized_dependent(materialized: materialized), :up) + run_migration(index_migration, :up) + expect { + run_migration(migration_for_update(materialized: materialized), :up) + }.to_not change { + indexes.on('dependent_greetings') + } + end + + it 'reverts' do + run_migration(migration_for_create(materialized: materialized), :up) + run_migration(migration_for_update(materialized: materialized), :up) + run_migration(migration_for_update(materialized: materialized), :down) + greeting = execute("SELECT * FROM dependent_greetings")[0]["greeting"] + expect(greeting).to eq 'hola' + end +end + + +describe "Dropping a view and its dependencies with cascade", :db do + around do |example| + with_view_definition :greetings, 1, "SELECT text 'hola' as greeting" do + with_view_definition :dependent_greetings, 1, "SELECT * from greetings" do + example.run + end + end + end + + it 'works' do + run_migration(migration_for_create, :up) + expect { + run_migration(migration_for_drop, :up) + }.to_not raise_error + end + + describe 'as part of updating a view' do + around do |example| + with_view_definition :greetings, 2, "SELECT text 'good day' AS greeting" do + example.run + end + end + + context 'with a non-materialized parent view' do + it_behaves_like "a cascading migration", false + end + + context 'with a materialized parent view' do + it_behaves_like "a cascading migration", true + end + end + + def migration_for_create(materialized: false) + eval <<-EOF + Class.new(migration_class) do + def change + create_view :greetings, materialized: #{materialized} + create_view :dependent_greetings + end + end + EOF + end + + def migration_for_create_materialized_dependent(materialized: false) + eval <<-EOF + Class.new(migration_class) do + def change + create_view :greetings, materialized: #{materialized} + create_view :dependent_greetings, materialized: true + end + end + EOF + end + + def migration_for_drop + Class.new(migration_class) do + def change + drop_view :greetings, revert_to_version: 1, cascade: true + end + end + end + + def migration_for_update(materialized: false) + eval <<-EOF + Class.new(migration_class) do + def change + update_view :greetings, + version: 2, + revert_to_version: 1, + cascade: true, + materialized: #{materialized} + end + end + EOF + end + + def index_migration + Class.new(migration_class) do + def change + add_index :dependent_greetings, :greeting + end + end + end + + +end diff --git a/spec/integration/revert_spec.rb b/spec/integration/revert_spec.rb index 2131e16f..af67d60e 100644 --- a/spec/integration/revert_spec.rb +++ b/spec/integration/revert_spec.rb @@ -52,23 +52,4 @@ def change end end - def migration_class - if Rails::VERSION::MAJOR >= 5 - ::ActiveRecord::Migration[5.0] - else - ::ActiveRecord::Migration - end - end - - def run_migration(migration, directions) - silence_stream(STDOUT) do - Array.wrap(directions).each do |direction| - migration.migrate(direction) - end - end - end - - def execute(sql) - ActiveRecord::Base.connection.execute(sql) - end end diff --git a/spec/scenic/command_recorder_spec.rb b/spec/scenic/command_recorder_spec.rb index 34c94594..5c5c7df5 100644 --- a/spec/scenic/command_recorder_spec.rb +++ b/spec/scenic/command_recorder_spec.rb @@ -37,6 +37,13 @@ expect { recorder.revert { recorder.drop_view(*args) } } .to raise_error(ActiveRecord::IrreversibleMigration) end + + it "raises when reverting with cascade set" do + args = [:users, { cascade: true, revert_to_version: 3 }] + + expect { recorder.revert { recorder.drop_view(*args) } } + .to raise_error(ActiveRecord::IrreversibleMigration) + end end describe "#update_view" do diff --git a/spec/scenic/statements_spec.rb b/spec/scenic/statements_spec.rb index 8c9161b9..77b888e6 100644 --- a/spec/scenic/statements_spec.rb +++ b/spec/scenic/statements_spec.rb @@ -65,7 +65,7 @@ module Scenic it "removes a view from the database" do connection.drop_view :name - expect(Scenic.database).to have_received(:drop_view).with(:name) + expect(Scenic.database).to have_received(:drop_view).with(:name, false) end end @@ -87,7 +87,7 @@ module Scenic connection.update_view(:name, version: 3) expect(Scenic.database).to have_received(:update_view) - .with(:name, definition.to_sql) + .with(:name, definition.to_sql, false) end it "updates a view from a text definition" do @@ -96,7 +96,7 @@ module Scenic connection.update_view(:name, sql_definition: sql_definition) expect(Scenic.database).to have_received(:update_view). - with(:name, sql_definition) + with(:name, sql_definition, false) end it "updates the materialized view in the database" do @@ -108,7 +108,7 @@ module Scenic connection.update_view(:name, version: 3, materialized: true) expect(Scenic.database).to have_received(:update_materialized_view). - with(:name, definition.to_sql) + with(:name, definition.to_sql, false) end it "raises an error if not supplied a version or sql_defintion" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f949b011..3297f7dc 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,10 +4,12 @@ require File.expand_path("../dummy/config/environment", __FILE__) require "support/generator_spec_setup" require "support/view_definition_helpers" +require "support/migration_helpers" RSpec.configure do |config| config.order = "random" config.include ViewDefinitionHelpers + config.include MigrationHelpers DatabaseCleaner.strategy = :transaction config.around(:each, db: true) do |example| diff --git a/spec/support/migration_helpers.rb b/spec/support/migration_helpers.rb new file mode 100644 index 00000000..176b6690 --- /dev/null +++ b/spec/support/migration_helpers.rb @@ -0,0 +1,27 @@ +module MigrationHelpers + + def migration_class + if Rails::VERSION::MAJOR >= 5 + ::ActiveRecord::Migration[5.0] + else + ::ActiveRecord::Migration + end + end + + def run_migration(migration, directions) + silence_stream(STDOUT) do + Array.wrap(directions).each do |direction| + migration.migrate(direction) + end + end + end + + def execute(sql) + connection.execute(sql) + end + + def connection + ActiveRecord::Base.connection + end + +end