scenic icon indicating copy to clipboard operation
scenic copied to clipboard

Add if_not_exists to create_view and if_exists to drop_view

Open serg-kovalev opened this issue 2 years ago • 4 comments

Provide an ability to pass if_not_exists param for view creation. It's handy to have it when you disable_ddl_transaction in Rails migration and add indexes concurrently. And similar functionality with if_exists param to drop a view.

For instance we have a migration like that:

class CreateSomeView < ActiveRecord::Migration[7.0]
  disable_ddl_transaction!

  def up
    create_view :mv_some_name, materialized: true

    safety_assured do
      add_index :mv_some_name, :receipt_uuid, unique: true, algorithm: :concurrently
      add_index :mv_some_name, :invoice_number, algorithm: :concurrently
    end
  end

  def down
    drop_view :mv_some_name, materialized: true
  end
end

As it's recommended to update indexes concurrently out of a transaction, those operations may accidentally fail. It happens very rarely, but we faced it several times. As a result, migration will stuck and it will require manual resolution.

The change that I made doesn't bring some level of uncertainty, I think. As by default, we do everything like we did before, but if we really need to make it a bit flexible, we now can do that.

Please let me know if it makes sense. Thank you in advance!

serg-kovalev avatar Feb 08 '23 12:02 serg-kovalev

@derekprior Hi! Hope you are well. Did not talk to you since we worked on NO DATA functionality. Could you please take a look at this PR? What do you think about adding functionality similar to what Rails has?

serg-kovalev avatar Feb 08 '23 13:02 serg-kovalev

Can you say more about this?

It's handy to have it when you disable_ddl_transaction in Rails migration and add indexes concurrently

Conceptually, one of the ideas of Scenic is that careful management of your schema using Scenic means you have certainty around your database state. IF NOT EXISTS implies uncertainty.

derekprior avatar Feb 08 '23 18:02 derekprior

Can you say more about this?

It's handy to have it when you disable_ddl_transaction in Rails migration and add indexes concurrently

Conceptually, one of the ideas of Scenic is that careful management of your schema using Scenic means you have certainty around your database state. IF NOT EXISTS implies uncertainty.

@derekprior Sure! For instance we have a migration like that:

class CreateSomeView < ActiveRecord::Migration[7.0]
  disable_ddl_transaction!

  def up
    create_view :mv_some_name, materialized: true

    safety_assured do
      add_index :mv_some_name, :receipt_uuid, unique: true, algorithm: :concurrently
      add_index :mv_some_name, :invoice_number, algorithm: :concurrently
    end
  end

  def down
    drop_view :mv_some_name, materialized: true
  end
end

As it's recommended to update indexes concurrently out of a transaction, those operations may accidentally fail. It happens very rarely, but we faced it several times. As a result, migration will stuck and it will require manual resolution.

The change that I made doesn't bring some level of uncertainty, I think. As by default, we do everything like we did before, but if we really need to make it a bit flexible, we now can do that.

Please let me know if it makes sense. Thank you in advance

serg-kovalev avatar Feb 09 '23 05:02 serg-kovalev

@derekprior Hey! Hope you are doing well. Have you had a chance to take a look?

serg-kovalev avatar Feb 21 '23 09:02 serg-kovalev