Refactor side_by_side materialized view creation

The initial implementation of side_by_side materialized view creation
worked but had a couple of issues that needed to be resolved and I
wanted to refactor the code for better maintainability.

* We had postgres-specific things in the `Scenic::Index` class, which is
  not part of the adapter API. The code was refactored to not rely on
  adding the schema name to this object.
* Index migration is different from index reapplication, and it felt
  like we were reusing `IndexReapplication` just to get access to the
  `SAVEPOINT` functionality in that class. I extracted `IndexCreation`
  which is now used by `IndexReapplication` and our new class,
  `IndexMigration`.
* Side-by-side logic was moved to a class of its own, `SideBySide`, for
  encapsulation purposes.
* Instead of conditionally hashing the view name in the case where the
  temporary name overflows the postgres identifier limit, we now always
  hash the temporary object names. This just keeps the code simpler and
  observed behavior from the outside identical no matter identifier
  length. This behavior is tested in the new `TemporaryName` class.
* Removed `rename_materialized_view` from the public API on the adapter,
  as I'd like to make sure that's something we want separate from this
  before we do something like that.
* Added `connection` to the public adapter UI for ease of use from our
  helper objects. Documented as internal use only.
* I added a number of tests for new and previously existing code.

Still to do:

1. I think we should consider enforcing that `side_by_side` updates are
   done in a transaction. Feels like it would be really bad if we failed
   somewhere after renaming indexes on the current view.
2. We should consider adding a final check to make sure no indexes or
   views are left behind that have our temporary name prefix. But I
   think that's probably being paranoid?
3. README documentation and generator support

lib/scenic/adapters/postgres.rb

@@ -4,6 +4,10 @@
 require_relative "postgres/indexes"
 require_relative "postgres/views"
 require_relative "postgres/refresh_dependencies"
+require_relative "postgres/side_by_side"
+require_relative "postgres/index_creation"
+require_relative "postgres/index_migration"
+require_relative "postgres/temporary_name"
 
 module Scenic
   # Scenic database adapters.
@@ -22,8 +26,6 @@ module Adapters
     # The methods are documented here for insight into specifics of how Scenic
     # integrates with Postgres and the responsibilities of {Adapters}.
     class Postgres
-      MAX_IDENTIFIER_LENGTH = 63
-
       # Creates an instance of the Scenic Postgres adapter.
       #
       # This is the default adapter for Scenic. Configuring it via
@@ -168,18 +170,14 @@ def create_materialized_view(name, sql_definition, no_data: false)
       def update_materialized_view(name, sql_definition, no_data: false, side_by_side: false)
         raise_unless_materialized_views_supported
 
+        if side_by_side && no_data
+          raise IncompatibleOptionsError, "side_by_side and no_data don't make sense together"
+        end
+
         if side_by_side
-          session_id = Time.now.to_i
-          new_name = generate_name name, "new_#{session_id}"
-          drop_name = generate_name name, "drop_#{session_id}"
-          IndexReapplication.new(connection: connection).on_side_by_side(
-            name, new_name, session_id
-          ) do
-            create_materialized_view(new_name, sql_definition, no_data: no_data)
-          end
-          rename_materialized_view(name, drop_name)
-          rename_materialized_view(new_name, name)
-          drop_materialized_view(drop_name)
+          SideBySide
+            .new(adapter: self, name: name, definition: sql_definition)
+            .update
         else
           IndexReapplication.new(connection: connection).on(name) do
             drop_materialized_view(name)
@@ -202,20 +200,6 @@ def drop_materialized_view(name)
         execute "DROP MATERIALIZED VIEW #{quote_table_name(name)};"
       end
 
-      # Renames a materialized view from {name} to {new_name}
-      #
-      # @param name The existing name of the materialized view in the database.
-      # @param new_name The new name to which it should be renamed
-      # @raise [MaterializedViewsNotSupportedError] if the version of Postgres
-      #   in use does not support materialized views.
-      #
-      # @return [void]
-      def rename_materialized_view(name, new_name)
-        raise_unless_materialized_views_supported
-        execute "ALTER MATERIALIZED VIEW #{quote_table_name(name)} " \
-                "RENAME TO #{quote_table_name(new_name)};"
-      end
-
       # Refreshes a materialized view from its SQL schema.
       #
       # This is typically called from application code via {Scenic.database}.
@@ -286,15 +270,19 @@ def populated?(name)
         end
       end
 
+      # A decorated ActiveRecord connection object with some Scenic-specific
+      # methods. Not intended for direct use outside of the Postgres adapter.
+      #
+      # @api private
+      def connection
+        Connection.new(connectable.connection)
+      end
+
       private
 
       attr_reader :connectable
       delegate :execute, :quote_table_name, to: :connection
 
-      def connection
-        Connection.new(connectable.connection)
-      end
-
       def raise_unless_materialized_views_supported
         unless connection.supports_materialized_views?
           raise MaterializedViewsNotSupportedError
@@ -315,16 +303,6 @@ def refresh_dependencies_for(name, concurrently: false)
           concurrently: concurrently
         )
       end
-
-      def generate_name(base, suffix)
-        candidate = "#{base}_#{suffix}"
-        if candidate.size <= MAX_IDENTIFIER_LENGTH
-          candidate
-        else
-          digest_length = MAX_IDENTIFIER_LENGTH - suffix.size - 1
-          "#{Digest::SHA256.hexdigest(base)[0...digest_length]}_#{suffix}"
-        end
-      end
     end
   end
 end

lib/scenic/adapters/postgres/errors.rb

@@ -21,6 +21,9 @@ def initialize
           super("Concurrent materialized view refreshes require Postgres 9.4 or newer")
         end
       end
+
+      class IncompatibleOptionsError < ArgumentError
+      end
     end
   end
 end

lib/scenic/adapters/postgres/index_creation.rb

@@ -0,0 +1,68 @@
+module Scenic
+  module Adapters
+    class Postgres
+      # Used to resiliently create indexes on a materialized view. If the index
+      # cannot be applied to the view (e.g. the columns don't exist any longer),
+      # we log that information and continue rather than raising an error. It is
+      # left to the user to judge whether the index is necessary and recreate
+      # it.
+      #
+      # Used when updating a materialized view to ensure the new version has all
+      # apprioriate indexes.
+      #
+      # @api private
+      class IndexCreation
+        # Creates the index creation object.
+        #
+        # @param connection [Connection] The connection to execute SQL against.
+        # @param speaker [#say] (ActiveRecord::Migration) The object used for
+        #   logging the results of creating indexes.
+        def initialize(connection:, speaker: ActiveRecord::Migration.new)
+          @connection = connection
+          @speaker = speaker
+        end
+
+        # Creates the provided indexes. If an index cannot be created, it is
+        # logged and the process continues.
+        #
+        # @param indexes [Array<Scenic::Index>] The indexes to create.
+        #
+        # @return [void]
+        def try_create(indexes)
+          Array(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)
+          end
+
+          if success
+            say "index '#{index.index_name}' on '#{index.object_name}' has been created"
+          else
+            say "index '#{index.index_name}' on '#{index.object_name}' is no longer valid and has been dropped."
+          end
+        end
+
+        def with_savepoint(name)
+          connection.execute("SAVEPOINT #{name}")
+          yield
+          connection.execute("RELEASE SAVEPOINT #{name}")
+          true
+        rescue
+          connection.execute("ROLLBACK TO SAVEPOINT #{name}")
+          false
+        end
+
+        def say(message)
+          subitem = true
+          speaker.say(message, subitem)
+        end
+      end
+    end
+  end
+end

lib/scenic/adapters/postgres/index_migration.rb

@@ -0,0 +1,61 @@
+module Scenic
+  module Adapters
+    class Postgres
+      # Used during side-by-side materialized view updates to migrate indexes
+      # from the original view to the new view.
+      #
+      # @api private
+      class IndexMigration
+        # Creates the index migration object.
+        #
+        # @param connection [Connection] The connection to execute SQL against.
+        # @param speaker [#say] (ActiveRecord::Migration) The object used for
+        #   logging the results of migrating indexes.
+        def initialize(connection:, speaker: ActiveRecord::Migration.new)
+          @connection = connection
+          @speaker = speaker
+        end
+
+        # Retreives the indexes on the original view, renames them to avoid
+        # collisions, retargets the indexes to the destination view, and then
+        # aookues the retargeted indexes.
+        #
+        # @param from [String] The name of the original view.
+        # @param to [String] The name of the destination view.
+        #
+        # @return [void]
+        def migrate(from:, to:)
+          source_indexes = Indexes.new(connection: connection).on(from)
+          retargeted_indexes = source_indexes.map { |i| retarget(i, to: to) }
+          source_indexes.each(&method(:rename))
+
+          IndexCreation
+            .new(connection: connection, speaker: speaker)
+            .try_create(retargeted_indexes)
+        end
+
+        private
+
+        attr_reader :connection, :speaker
+
+        def retarget(index, to:)
+          new_definition = index.definition.sub(
+            /ON (.*)\.#{index.object_name}/,
+            'ON \1.' + to + ' '
+          )
+
+          Scenic::Index.new(
+            object_name: to,
+            index_name: index.index_name,
+            definition: new_definition,
+          )
+        end
+
+        def rename(index)
+          temporary_name = TemporaryName.new(index.index_name).to_s
+          connection.rename_index(index.object_name, index.index_name, temporary_name)
+        end
+      end
+    end
+  end
+end

lib/scenic/adapters/postgres/index_reapplication.rb

@@ -32,51 +32,14 @@ def on(name)
 
           yield
 
-          indexes.each(&method(:try_index_create))
-        end
-
-        def on_side_by_side(name, new_table_name, temporary_id)
-          indexes = Indexes.new(connection: connection).on(name)
-          indexes.each_with_index do |index, i|
-            old_name = "predrop_index_#{temporary_id}_#{i}"
-            connection.rename_index(name, index.index_name, old_name)
-          end
-          yield
-          indexes.each do |index|
-            try_index_create(index.with_other_object_name(new_table_name))
-          end
+          IndexCreation
+            .new(connection: connection, speaker: speaker)
+            .try_create(indexes)
         end
 
         private
 
         attr_reader :connection, :speaker
-
-        def try_index_create(index)
-          success = with_savepoint(index.index_name) do
-            connection.execute(index.definition)
-          end
-
-          if success
-            say "index '#{index.index_name}' on '#{index.object_name}' has been recreated"
-          else
-            say "index '#{index.index_name}' on '#{index.object_name}' is no longer valid and has been dropped."
-          end
-        end
-
-        def with_savepoint(name)
-          connection.execute("SAVEPOINT #{name}")
-          yield
-          connection.execute("RELEASE SAVEPOINT #{name}")
-          true
-        rescue
-          connection.execute("ROLLBACK TO SAVEPOINT #{name}")
-          false
-        end
-
-        def say(message)
-          subitem = true
-          speaker.say(message, subitem)
-        end
       end
     end
   end

lib/scenic/adapters/postgres/indexes.rb

@@ -27,7 +27,6 @@ def indexes_on(name)
             SELECT
               t.relname as object_name,
               i.relname as index_name,
-              n.nspname as schema_name,
               pg_get_indexdef(d.indexrelid) AS definition
             FROM pg_class t
             INNER JOIN pg_index d ON t.oid = d.indrelid
@@ -46,7 +45,6 @@ def index_from_database(result)
             object_name: result["object_name"],
             index_name: result["index_name"],
             definition: result["definition"],
-            schema_name: result["schema_name"]
           )
         end
       end

lib/scenic/adapters/postgres/side_by_side.rb

@@ -0,0 +1,38 @@
+module Scenic
+  module Adapters
+    class Postgres
+      class SideBySide
+        def initialize(adapter:, name:, definition:, speaker: ActiveRecord::Migration.new)
+          @adapter = adapter
+          @name = name
+          @definition = definition
+          @temporary_name = TemporaryName.new(name).to_s
+          @speaker = speaker
+        end
+
+        def update
+          adapter.create_materialized_view(temporary_name, definition)
+
+          IndexMigration
+            .new(connection: adapter.connection, speaker: speaker)
+            .migrate(from: name, to: temporary_name)
+
+          adapter.drop_materialized_view(name)
+          rename_materialized_view(temporary_name, name)
+        end
+
+        private
+
+        attr_reader :adapter, :name, :definition, :temporary_name, :speaker
+
+        def connection
+          adapter.connection
+        end
+
+        def rename_materialized_view(from, to)
+          connection.execute("ALTER MATERIALIZED VIEW #{from} RENAME TO #{to}")
+        end
+      end
+    end
+  end
+end

lib/scenic/adapters/postgres/temporary_name.rb

@@ -0,0 +1,34 @@
+module Scenic
+  module Adapters
+    class Postgres
+      # Generates a temporary object name used internally by Scenic. This is
+      # used during side-by-side materialized view updates to avoid naming
+      # collisions. The generated name is based on a SHA1 hash of the original
+      # which ensures we do not exceed the 63 character limit for object names.
+      #
+      # @api private
+      class TemporaryName
+        # The prefix used for all temporary names.
+        PREFIX = "_scenic_sbs_".freeze
+
+        # Creates a new temporary name object.
+        #
+        # @param name [String] The original name to base the temporary name on.
+        def initialize(name)
+          @name = name
+          @salt = SecureRandom.hex(4)
+          @temporary_name = "#{PREFIX}#{Digest::SHA1.hexdigest(name + salt)}"
+        end
+
+        # @return [String] The temporary name.
+        def to_s
+          temporary_name
+        end
+
+        private
+
+        attr_reader :name, :temporary_name, :salt
+      end
+    end
+  end
+end