Skip to content

feat: postgres migration table existence check#860

Merged
mfridman merged 8 commits intomasterfrom
mf/gh461-store-controller
Nov 24, 2024
Merged

feat: postgres migration table existence check#860
mfridman merged 8 commits intomasterfrom
mf/gh461-store-controller

Conversation

@mfridman
Copy link
Copy Markdown
Collaborator

@mfridman mfridman commented Nov 23, 2024

Related to #461

This PR introduces an extension mechanism for Store implementations to optimize database operations. The first extension is table existence checking, which improves performance and reduces error logging noise.

New Interface

TableExists(ctx context.Context, db database.DBTxConn) (bool, error)

Benefits

  • More efficient table existence checking
  • Eliminates noisy error logs like:

    ERROR: relation "goose_db_version" does not exist at character 36

Implementation

The postgres implementation uses native pg_tables queries, serving as a template for other dialects.

I initially tried out the store controller in #752, and I didn't mind it. But decided to keep it ./internal since only the guts of goose should care about this.

The only thing end-users need to know is that they can implement the above method on their custom Store. And for maintainers, all we need to do to add support is follow the postgres example.

}

func (p *Postgres) TableExists(tableName string) string {
schemaName, tableName := parseTableIdentifier(tableName)
Copy link
Copy Markdown
Collaborator Author

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.

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)
Copy link
Copy Markdown
Collaborator Author

@mfridman mfridman Nov 23, 2024

Choose a reason for hiding this comment

The 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 Store interface.

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.

type customStoreSQLite3 struct {
database.Store
}
var _ database.StoreExtender = (*customStoreSQLite3)(nil)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels much nicer.

@mfridman mfridman merged commit 48c4946 into master Nov 24, 2024
@mfridman mfridman deleted the mf/gh461-store-controller branch November 24, 2024 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant