-
Notifications
You must be signed in to change notification settings - Fork 528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: postgres migration table existence check #860
Changes from 2 commits
5252c5c
e228fc3
b66f772
4796eb9
a14c4a6
6feb4c9
da8e21f
c41b2b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package controller | ||
|
||
import ( | ||
"context" | ||
"errors" | ||
|
||
"github.com/pressly/goose/v3/database" | ||
) | ||
|
||
// A StoreController is used by the goose package to interact with a database. This type is a | ||
// wrapper around the Store interface, but can be extended to include additional (optional) methods | ||
// that are not part of the core Store interface. | ||
type StoreController struct{ database.Store } | ||
|
||
var _ database.Store = (*StoreController)(nil) | ||
|
||
// NewStoreController returns a new StoreController that wraps the given Store. | ||
// | ||
// If the Store implements the following optional methods, the StoreController will call them as | ||
// appropriate: | ||
// | ||
// - TableExists(context.Context, DBTxConn) (bool, error) | ||
// | ||
// If the Store does not implement a method, it will either return a [errors.ErrUnsupported] error | ||
// or fall back to the default behavior. | ||
func NewStoreController(store database.Store) *StoreController { | ||
return &StoreController{store} | ||
} | ||
|
||
// TableExists is an optional method that checks if the version table exists in the database. It is | ||
// recommended to implement this method if the database supports it, as it can be used to optimize | ||
// certain operations. | ||
func (c *StoreController) TableExists(ctx context.Context, db database.DBTxConn) (bool, error) { | ||
if t, ok := c.Store.(interface { | ||
TableExists(ctx context.Context, db database.DBTxConn) (bool, error) | ||
}); ok { | ||
return t.TableExists(ctx, db) | ||
} | ||
return false, errors.ErrUnsupported | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -296,25 +296,29 @@ func (p *Provider) tryEnsureVersionTable(ctx context.Context, conn *sql.Conn) er | |
b := retry.NewConstant(1 * time.Second) | ||
b = retry.WithMaxRetries(3, b) | ||
return retry.Do(ctx, b, func(ctx context.Context) error { | ||
if e, ok := p.store.(interface { | ||
TableExists(context.Context, database.DBTxConn, string) (bool, error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The whole point of this PR was to make this a "well-defined" method part of the extended By having this inlined, makes it difficult to understand where else in the codebase there might be such hidden interfaces. But with the concept of the "controller" there's a single place where users (and us) can look to discover these. |
||
}); ok { | ||
exists, err := e.TableExists(ctx, conn, p.store.Tablename()) | ||
if err != nil { | ||
return fmt.Errorf("failed to check if version table exists: %w", err) | ||
} | ||
if exists { | ||
return nil | ||
} | ||
} else { | ||
// This chicken-and-egg behavior is the fallback for all existing implementations of the | ||
// Store interface. We check if the version table exists by querying for the initial | ||
// version, but the table may not exist yet. It's important this runs outside of a | ||
// transaction to avoid failing the transaction. | ||
exists, err := p.store.TableExists(ctx, conn) | ||
if err == nil && exists { | ||
return nil | ||
} else if err != nil && errors.Is(err, errors.ErrUnsupported) { | ||
// Fallback strategy for checking table existence: | ||
// | ||
// When direct table existence checks aren't supported, we attempt to query the initial | ||
// migration (version 0). This approach has two implications: | ||
// | ||
// 1. If the table exists, the query succeeds and confirms existence | ||
// 2. If the table doesn't exist, the query fails and generates an error log | ||
// | ||
// Note: This check must occur outside any transaction, as a failed query would | ||
// otherwise cause the entire transaction to roll back. The error logs generated by this | ||
// approach are expected and can be safely ignored. | ||
if res, err := p.store.GetMigration(ctx, conn, 0); err == nil && res != nil { | ||
return nil | ||
} | ||
// Fallthrough to create the table. | ||
} else if err != nil { | ||
return fmt.Errorf("failed to check if version table exists: %w", err) | ||
} | ||
|
||
if err := beginTx(ctx, conn, func(tx *sql.Tx) error { | ||
if err := p.store.CreateVersionTable(ctx, tx); err != nil { | ||
return err | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should handle cases where the table name contains the schema name; otherwise, assumed
public.