Introduce DBAction interface and refactoring of column dropping

[kvch]

This PR is part of a refactoring effort. My goal is to unify all database actions pgroll is executing. Example actions include dropping columns, renaming columns, creating and dropping triggers, creating constraints, etc.

We can define a list of these actions in a migration phase and run them sequentially. The following example is a Complete phase of an imaginary migration:

func Complete(ctx context.Context) error {
    actions := []DBAction{
        NewDropColumnAction(conn, tableName, columnName),
        NewRenameColumnAction(conn, tableName, newName, oldName),
        NewDropColumnAction(conn, tableName, CNeedsBackfill),
        NewDropTriggerAction(conn, triggerName),
    }

    for _, action := actions {
       if err := action.Execute(ctx); err != nil {
           return err
       }
    }
}

In this step I implemented column dropping as an action. In later PRs I would like to add creating and dropping triggers, functions, etc.

[exekias]

@kvch code change LGTM but I would wait on the rest of the team to see if this is the desired path! my personal view:

I kind of like that operations are self-contained for the most part, with some helper functions for complex stuff like backfills. This pattern would force us to write the statements elsewhere, hiding the operation implementation a bit. Not a big deal for me, but still...

[kvch]

The operations are getting less and less self-contained, because we are adding more and more complexity as go beyond the naive implementation of each migration. Also, some of the operations have overlap like create_constraint and create_table or adding a unique constraint and adding a unique index. Just to list a few examples.

Furthermore, we already have createUniqueIndexConcurrently, alterSequenceOwner, RenameColumn etc. These are already actions, and we are organically going towards this architecture. But instead of building an "accidental" architecture, I would like to preserve the clarity of pgroll and consciously prepare the codebase for the future. The operations we perform in each migration phase fits perfectly the command design pattern. This is my attempt to create reusable "commands"/"actions" without the fluff. Thus, we can focus on putting together a migration from existing building blocks, instead of concatenating the same SQL strings in multiple operations.

[exekias]

👍 thank you for the detailed explanation! no strong opinion on my side, to me whatever @andrew-farries and you decide works for me!

[andrew-farries]

I agree with the direction of this change but I think we can go further:

For at least the Start phase, and possibly also for Complete and Rollback the method could return the list of DBActions to be executed, rather than having the methods execute the actions directly.

This would allow us to more easily re-order and squash steps in multi-operation migations, for example allowing two operations to modify the same column - this is currently unsupported as each operation independently tries to duplicate the column.

WDYT @kvch ?

Pull Request Overview

This PR introduces a new DBAction interface with its dropColumnAction implementation to streamline and unify various database migration operations. The changes refactor multiple migration operations to use NewDropColumnAction for dropping columns, which improves consistency and modularity in how migration steps are executed.

• Introduces a DBAction interface and dropColumnAction for column dropping.
• Refactors several migration operations (add, drop, alter, and constraint operations) to use NewDropColumnAction.
• Enhances consistency in executing SQL column drop operations across different migration files.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/migrations/dbactions.go	New DBAction interface and dropColumnAction implementation
pkg/migrations/op_add_column.go	Replaced direct ExecContext call with dropColumnAction usage
pkg/migrations/op_drop_constraint.go	Refactored drop column logic to use dropColumnAction
pkg/migrations/op_create_constraint.go	Updated drop column and backfill removal with dropColumnAction
pkg/migrations/op_drop_column.go	Refactored drop column steps to use dropColumnAction
pkg/migrations/op_drop_multicolumn_constraint.go	Updated multiple column drop and backfill removal with dropColumnAction
pkg/migrations/op_alter_column.go	Replaced direct drops with dropColumnAction usage in complete and rollback steps

[kvch]

I like this approach. To keep the review burden and the changeset small, I would like to transform all existing database actions to the common format in different PRs. Then in a separate PR I will implement the new interface for Start, Complete and Rollback. Do that work for you?

[andrew-farries]

That works for me 👍