Add support to replace functions with dependencies

[sobrinho]

This will add support to update functions without dropping and recreating dependencies.

[sobrinho]

@teoljungberg in other words, today we have to do this:

def up
  drop_trigger :blah, on: :blah
  update_function :blah, version: 2
  create_trigger :blah, on: :blah
end

def down
  drop_trigger :blah, on: :blah
  update_function :blah, version: 1
  create_trigger :blah, on: :blah
end

And after the PR, we just do:

def change
  update_function :blah, version: 2, revert_to_version: 1, replace: true
end

[bf4]

I was just hacking the gem for the feature in this PR. I'm happy to take it over if OP @sobrinho is taking a break.

Basic impl looks good to me

[sobrinho]

I will do the requested changes in a couple of hours :)

[bf4]

I'm happy to take this over if it's gonna be stale much longer. I've added some work arounds in my app, but I'd like to use it :)

[teoljungberg]

Please do @bf4. I'd like this feature too.

[sobrinho]

@teoljungberg would you review it again?

Sorry about the delay, I have been busy in the last days.

[teoljungberg]

As we can do this today, in a bit of a clunkier fashion, I think it's fine and we can leave this as is.

[sobrinho]

@teoljungberg this is a small effort in terms of code and maintenance in exchange of a big convenience for heavy users. Would you reconsider it?

[teoljungberg]

I'd love to hear from others to reconsider it, and as I said

As we can do this today, in a bit of a clunkier fashion, I think it's fine and we can leave this as is.

I don't see this as something that needs to be considered right now. Without having more input.

[bf4]

@teoljungberg for me, I've worked around this by writing my own update functions as described in #63

  # Could be simplified after https://github.com/teoljungberg/fx/pull/52 is merged
  def db_update_function(function_name, version:, revert_to_version:, replace: true)
    if replace
      reversible do |dir|
        dir.up do
          safely_execute function_sql(function_name, version: version)
        end
        dir.down do
          safely_execute function_sql(function_name, version: revert_to_version)
        end
      end
    else
      safety_assured do
        update_function function_name version: version, revert_to_version: revert_to_version
      end
    end
  end

if this PR (or something like it) were merged, even as an opt-in (rather than opt-out), I could change my code to

  def db_update_function(function_name, version:, revert_to_version:, replace: true)
    safety_assured do
      update_function function_name version: version, revert_to_version: revert_to_version, replace: replace
    end
  end

because we use CREATE OR REPLACE FUNCTION for all our function files

maybe call drop_first: true as your default on update_function to make it opt in and descriptive?

[teoljungberg]

Thanks for the additional input @bf4 - good to have for future context.

[trcarden]

I'd love to hear from others to reconsider it, and as I said

As we can do this today, in a bit of a clunkier fashion, I think it's fine and we can leave this as is.

I don't see this as something that needs to be considered right now. Without having more input.

@teoljungberg here is a bit more input:

• We use the functions above in db views (for various reasons)
• Dropping a function would require coordination across all the views that depend on it
• Adding replacement support so we don't have to drop and recreate views (which can be expensive, especially if they are materialized and large) would be a win
• This PR, while originally built to support triggers, could also be used for all other dependent object types

I am going to workaround by just calling create_function again (much like this PR does) directly since the SQL for creation has the replacement statements built in.