-
Notifications
You must be signed in to change notification settings - Fork 526
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 all 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,33 @@ | ||
package database | ||
|
||
import "context" | ||
|
||
// StoreExtender is an extension of the Store interface that provides optional optimizations and | ||
// database-specific features. While not required by the core goose package, implementing these | ||
// methods can improve performance and functionality for specific databases. | ||
// | ||
// IMPORTANT: This interface may be expanded in future versions. Implementors MUST be prepared to | ||
// update their implementations when new methods are added, either by implementing the new | ||
// functionality or returning [errors.ErrUnsupported]. | ||
// | ||
// The goose package handles these extended capabilities through a [controller.StoreController], | ||
// which automatically uses optimized methods when available while falling back to default behavior | ||
// when they're not implemented. | ||
// | ||
// Example usage to verify implementation: | ||
// | ||
// var _ StoreExtender = (*CustomStoreExtended)(nil) | ||
// | ||
// In short, it's exported to allows implementors to have a compile-time check that they are | ||
// implementing the interface correctly. | ||
type StoreExtender interface { | ||
Store | ||
|
||
// TableExists checks if the migrations table exists in the database. Implementing this method | ||
// allows goose to optimize table existence checks by using database-specific system catalogs | ||
// (e.g., pg_tables for PostgreSQL, sqlite_master for SQLite) instead of generic SQL queries. | ||
// | ||
// Return [errors.ErrUnsupported] if the database does not provide an efficient way to check | ||
// table existence. | ||
TableExists(ctx context.Context, db DBTxConn) (bool, error) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
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.StoreExtender = (*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} | ||
} | ||
|
||
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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -745,15 +745,17 @@ func TestGoMigrationPanic(t *testing.T) { | |
|
||
func TestCustomStoreTableExists(t *testing.T) { | ||
t.Parallel() | ||
|
||
db := newDB(t) | ||
store, err := database.NewStore(database.DialectSQLite3, goose.DefaultTablename) | ||
require.NoError(t, err) | ||
p, err := goose.NewProvider("", newDB(t), newFsys(), | ||
goose.WithStore(&customStoreSQLite3{store}), | ||
) | ||
require.NoError(t, err) | ||
_, err = p.Up(context.Background()) | ||
require.NoError(t, err) | ||
for i := 0; i < 2; i++ { | ||
p, err := goose.NewProvider("", db, newFsys(), | ||
goose.WithStore(&customStoreSQLite3{store}), | ||
) | ||
require.NoError(t, err) | ||
_, err = p.Up(context.Background()) | ||
require.NoError(t, err) | ||
} | ||
} | ||
|
||
func TestProviderApply(t *testing.T) { | ||
|
@@ -842,14 +844,14 @@ func TestPending(t *testing.T) { | |
}) | ||
} | ||
|
||
type customStoreSQLite3 struct { | ||
database.Store | ||
} | ||
var _ database.StoreExtender = (*customStoreSQLite3)(nil) | ||
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. This feels much nicer. |
||
|
||
type customStoreSQLite3 struct{ database.Store } | ||
|
||
func (s *customStoreSQLite3) TableExists(ctx context.Context, db database.DBTxConn, name string) (bool, error) { | ||
q := `SELECT EXISTS (SELECT 1 FROM sqlite_master WHERE type='table' AND name=$1) AS table_exists` | ||
func (s *customStoreSQLite3) TableExists(ctx context.Context, db database.DBTxConn) (bool, error) { | ||
q := `SELECT EXISTS (SELECT 1 FROM sqlite_master WHERE type='table' AND name=?) AS table_exists` | ||
var exists bool | ||
if err := db.QueryRowContext(ctx, q, name).Scan(&exists); err != nil { | ||
if err := db.QueryRowContext(ctx, q, s.Tablename()).Scan(&exists); err != nil { | ||
return false, err | ||
} | ||
return exists, nil | ||
|
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.