add indexes at view creation instead of at view renaming

.rubocop.yml

@@ -65,7 +65,6 @@ Style/BlockDelimiters:
 Style/CollectionMethods:
   Enabled: true
   PreferredMethods:
-    find: find
     inject: reduce
     collect: map
     find_all: select

README.md

@@ -228,34 +228,44 @@ populate the next version of the view in a previous release. So you will have
 to follow these steps:
 
 1. Create a new materialized view
-   `rails generate scenic:view table_name_next --materialized --no-data`
-   that will take improvement of your future view `table_name`, you can 
-   copy-paste the content of the last version of `table_name` as a starter.
+   `rails generate scenic:view table_names_next --materialized --no-data`
+   that will take improvement of your future view `table_namse`. you can
+   copy-paste the content of the last version of `table_names` as a starter
+   and you can also add the option `copy_index_from: :table_names` to easily add
+   all indexes of `table_names`.
+   ```ruby
+   def change
+     create_view :table_names_nexts,
+       version: 1,
+       materialized: { no_data: true, copy_index_from: :table_names }
+   end
+   ```
 2. Deploy and apply this migration
 3. Refresh the view within a task or in the Rails console
-   `Scenic.database.refresh_materialized_view(:table_name_nexts, concurrently: false)`
+   `Scenic.database.refresh_materialized_view(:table_names_nexts, concurrently: false)`
 4. Use that view by removing the previous and renaming the next one in a single
    migration 
-   `rails generate scenic:view table_name --materialized --rename table_name_next`
+   `rails generate scenic:view table_names --materialized --rename table_names_next`
    and edit the migration to change `rename_view` by `replace_view`:
    ```ruby
    def change
-     replace_view :table_name_nexts, :table_names,
+     replace_view :table_names_nexts, :table_names,
        version: 1,
        revert_to_version: 1,
        materialized: true
    end
    ```
 
 `replace_view` will internaly do:
-1. store indexes of `table_names`
-2. remove `table_names`
-3. rename `table_names_next` to `table_names`
-4. ensure that the definition on the database is the same as the one in 
-   `db/views` folder
-5. rename indexes by replacing `table_names_next` by `table_names` in 
-   their names
-6. applied valid stored indexes on the new version of `table_names`
+```ruby
+drop_view(:table_names, revert_to_version: 1, materialized: true)
+rename_view(
+  :table_names_nexts, :table_names,
+  version: 1, revert_to_version: 1, materialized: { rename_indexes: true }
+)
+```
+So you can use this last version if you need to perform some operations between
+the drop and the rename.
 
 ## I don't need this view anymore. Make it go away.
 

lib/scenic/adapters/postgres.rb

@@ -140,21 +140,31 @@ def rename_view(from_name, to_name)
       # @param no_data [Boolean] Default: false. Set to true to create
       #   materialized view without running the associated query. You will need
       #   to perform a non-concurrent refresh to populate with data.
+      # @param copy_indexes_from [String] Default: false. Name of another view
+      #   to copy indexes from. Useful when used as a first step before
+      #   `replace_materialized_view`.
       #
       # This is typically called in a migration via {Statements#create_view}.
       #
       # @raise [MaterializedViewsNotSupportedError] if the version of Postgres
       #   in use does not support materialized views.
       #
       # @return [void]
-      def create_materialized_view(name, sql_definition, no_data: false)
+      def create_materialized_view(
+        name, sql_definition,
+        no_data: false, copy_indexes_from: false
+      )
         raise_unless_materialized_views_supported
 
-        execute <<-SQL
-  CREATE MATERIALIZED VIEW #{quote_table_name(name)} AS
-  #{sql_definition.rstrip.chomp(';')}
-  #{'WITH NO DATA' if no_data};
+        execute <<~SQL
+          CREATE MATERIALIZED VIEW #{quote_table_name(name)} AS
+          #{sql_definition.rstrip.chomp(';')}
+          #{'WITH NO DATA' if no_data};
         SQL
+        if copy_indexes_from
+          IndexReapplication.new(connection: connection)
+            .on(name, from: copy_indexes_from) {}
+        end
       end
 
       # Updates a materialized view in the database.
@@ -200,10 +210,8 @@ def update_materialized_view(name, sql_definition, no_data: false)
       def replace_materialized_view(from_name, to_name)
         raise_unless_materialized_views_supported
 
-        IndexReapplication.new(connection: connection).on(to_name) do
-          drop_materialized_view(to_name)
-          rename_materialized_view(from_name, to_name, rename_indexes: true)
-        end
+        drop_materialized_view(to_name)
+        rename_materialized_view(from_name, to_name, rename_indexes: true)
       end
 
       # Drops a materialized view in the database
@@ -246,7 +254,9 @@ def rename_materialized_view(from_name, to_name, rename_indexes: false)
             .map(&:index_name)
             .select { |name| name.match?(from_name) }
             .each do |name|
-              rename_index to_name, name, name.sub(from_name, to_name)
+              connection.rename_index(
+                to_name, name, name.sub(from_name, to_name)
+              )
             end
         end
       end
@@ -333,8 +343,7 @@ def view_with_similar_definition?(definition)
 
       attr_reader :connectable
       delegate(
-        :execute, :quote, :quote_table_name, :rename_index,
-        :select_value, :transaction,
+        :execute, :quote, :quote_table_name, :select_value, :transaction,
         to: :connection
       )
 

lib/scenic/adapters/postgres/index_reapplication.rb

@@ -27,12 +27,15 @@ def initialize(connection:, speaker: ActiveRecord::Migration)
         # @yield Operations to perform before reapplying indexes.
         #
         # @return [void]
-        def on(name)
-          indexes = Indexes.new(connection: connection).on(name)
+        def on(name, from: name)
+          indexes = Indexes.new(connection: connection).on(from)
 
           yield
 
-          indexes.each(&method(:try_index_create))
+          indexes
+            .map(&method(:change_index_object_name).curry[from, name])
+            .tap {|i| p i }
+            .each(&method(:try_index_create))
         end
 
         private
@@ -51,12 +54,25 @@ def try_index_create(index)
           end
         end
 
+        def change_index_object_name(from, to, index)
+          return index if from == to
+
+          Scenic::Index.new(
+            object_name: to,
+            index_name: index.index_name.sub(from.to_s, to.to_s),
+            definition: index.definition
+              .sub(/(\w*)#{from}(\w*) ON/, "\\1#{to}\\2")
+              .sub(/ON (\w+\.)?#{from}/, "ON \\1#{to}"),
+          )
+        end
+
         def with_savepoint(name)
           connection.execute("SAVEPOINT #{name}")
           yield
           connection.execute("RELEASE SAVEPOINT #{name}")
           true
-        rescue
+        rescue => e
+          p e
           connection.execute("ROLLBACK TO SAVEPOINT #{name}")
           false
         end

lib/scenic/command_recorder.rb

@@ -24,18 +24,7 @@ def invert_update_view(args)
     end
 
     def invert_replace_view(args)
-      if StatementArguments.new(args).materialized?
-        raise ActiveRecord::IrreversibleMigration, <<~MSG
-          This migration uses replace_view for materialized view,
-          which is not automatically reversible due to indexes moving
-          and renaming.
-          To make the migration reversible you can either:
-          1. Define #up and #down methods in place of the #change method.
-          2. Use the #reversible method to define reversible behavior.
-        MSG
-      else
-        perform_scenic_inversion(:replace_view, args)
-      end
+      perform_scenic_inversion(:replace_view, args)
     end
 
     def invert_rename_view(args)

lib/scenic/statements.rb

@@ -12,9 +12,11 @@ module Statements
     # @param sql_definition [String] The SQL query for the view schema. An error
     #   will be raised if `sql_definition` and `version` are both set,
     #   as they are mutually exclusive.
-    # @param materialized [Boolean, Hash] Set to true to create a materialized
-    #   view. Set to { no_data: true } to create materialized view without
-    #   loading data. Defaults to false.
+    # @param materialized [Boolean, Hash] Default: false.
+    #   Set to true to create a materialized view.
+    #   Set to { no_data: true } to not populate the view.
+    #   Set to { copy_indexes_from: "other_view_name" } to copy indexes from
+    #     that view. Useful when used as a first step before `replace_view`.
     # @return The database response from executing the create statement.
     #
     # @example Create from `db/views/searches_v02.sql`
@@ -44,6 +46,7 @@ def create_view(name, version: nil, sql_definition: nil, materialized: false)
           name,
           sql_definition,
           no_data: no_data(materialized),
+          copy_indexes_from: copy_indexes_from(materialized),
         )
       else
         database.create_view(name, sql_definition)
@@ -214,6 +217,14 @@ def no_data(materialized)
       end
     end
 
+    def copy_indexes_from(materialized)
+      if materialized.is_a?(Hash)
+        materialized.fetch(:copy_indexes_from, false)
+      else
+        false
+      end
+    end
+
     def rename_indexes(materialized)
       if materialized.is_a?(Hash)
         materialized.fetch(:rename_indexes, false)

spec/scenic/adapters/postgres_spec.rb

@@ -41,6 +41,26 @@ module Adapters
           expect(view.materialized).to eq true
         end
 
+        it "copy indexes from another view" do
+          adapter = Postgres.new
+          connection = ActiveRecord::Base.connection
+
+          adapter.create_materialized_view(
+            "greetings",
+            "SELECT text 'hi' AS greeting; \n",
+          )
+          connection.add_index :greetings, :greeting
+          adapter.create_materialized_view(
+            "greetings_nexts",
+            "SELECT text 'hello' AS greeting; \n",
+            copy_indexes_from: :greetings,
+          )
+
+          indexes = Postgres::Indexes.new(connection: connection)
+          index_names = indexes.on("greetings_nexts").map(&:index_name)
+          expect(index_names).to include("index_greetings_next_on_greeting")
+        end
+
         it "raises an exception if the version of PostgreSQL is too old" do
           connection = double("Connection", supports_materialized_views?: false)
           connectable = double("Connectable", connection: connection)
@@ -152,12 +172,13 @@ module Adapters
 
         it "successfully renames materialized view indexes" do
           adapter = Postgres.new
+          connection = ActiveRecord::Base.connection
 
           adapter.create_materialized_view(
             "greetings",
             "SELECT text 'hi' AS greeting",
           )
-          ActiveRecord::Base.connection.add_index "greetings", "greeting"
+          connection.add_index :greetings, :greeting
           adapter.rename_materialized_view(
             "greetings",
             "renamed",

spec/scenic/statements_spec.rb

@@ -58,7 +58,12 @@ module Scenic
         connection.create_view(:views, version: 1, materialized: true)
 
         expect(Scenic.database).to have_received(:create_materialized_view)
-          .with(:views, definition.to_sql, no_data: false)
+          .with(
+            :views,
+            definition.to_sql,
+            no_data: false,
+            copy_indexes_from: false,
+          )
       end
     end
 
@@ -74,7 +79,52 @@ module Scenic
         )
 
         expect(Scenic.database).to have_received(:create_materialized_view)
-          .with(:views, definition.to_sql, no_data: true)
+          .with(
+            :views, definition.to_sql,
+            no_data: true, copy_indexes_from: false
+          )
+      end
+    end
+
+    describe "create_view :materialized and copy indexes" do
+      it "sends the create_materialized_view message" do
+        definition = instance_double("Scenic::Definition", to_sql: "definition")
+        allow(Definition).to receive(:new).and_return(definition)
+
+        connection.create_view(
+          :views,
+          version: 1,
+          materialized: { copy_indexes_from: :other_views },
+        )
+
+        expect(Scenic.database).to have_received(:create_materialized_view)
+          .with(
+            :views,
+            definition.to_sql,
+            no_data: false,
+            copy_indexes_from: :other_views,
+          )
+      end
+    end
+
+    describe "create_view :materialized with :no_data and copy indexes" do
+      it "sends the create_materialized_view message" do
+        definition = instance_double("Scenic::Definition", to_sql: "definition")
+        allow(Definition).to receive(:new).and_return(definition)
+
+        connection.create_view(
+          :views,
+          version: 1,
+          materialized: { no_data: true, copy_indexes_from: :other_views },
+        )
+
+        expect(Scenic.database).to have_received(:create_materialized_view)
+          .with(
+            :views,
+            definition.to_sql,
+            no_data: true,
+            copy_indexes_from: :other_views,
+          )
       end
     end