Allow init goose version table before querying it.

[ori-shalom]

When running goose for the first time on a clean DB the following lines are printed to the DB log (using Postgres):

ERROR:  relation "goose_db_version" does not exist at character 36
STATEMENT:  SELECT version_id, is_applied from goose_db_version ORDER BY id DESC

The reason is that EnsureDBVersion is always trying first to query goose_db_version and only if it fails it creates the version table.

func EnsureDBVersion(db *sql.DB) (int64, error) {
	rows, err := GetDialect().dbVersionQuery(db)
	if err != nil {
		return 0, createVersionTable(db)
	}
	...

While this logic makes sense, we are concerned that our customers running our service will see this error in their database and think this is a bug.

In our service, we have a logic before running any migration that checks if the DB is "clean"/has our tables/has tables we don't know (used to prevent installing our service to a dirty/used database).

I wonder if it is possible to make createVersionTable public so we can call it when we identify the DB is clean and, by doing so, avoid the error in the DB log.

If you are open to this change, I'll be happy to open a PR adding this.

[mfridman]

I could be wrong, but it feels like this should be addressed in goose for everyone. I think there is a more robust way to ask the question "do we need to create the version table?".

In the current implementation, we query a table that may or may not exist, discard the error wholesale, and then attempt to create the table on any error.

Ideally, we could modify this query to check whether the table exists by utilizing the information_schema or pg_catalog (and the equivalent for other databases).

upper/db does this for various databases to resolve collections to tables, and it works quite nicely (from my experience). Example:

https://github.com/upper/db/blob/8a3fe0cb3bf37131aaa9c9f7c73d3e6517ef545f/adapter/postgresql/database.go#L45-L48

This seems like a safer and more explicit approach, maybe @VojtechVitek has some thoughts here?

[ori-shalom]

I initially thought it might be better not to add this check to goose to avoid the additional query to the DB if some users don't care about the error and prefer fewer roundtrips.

If you think this query should be part of goose then I don't mind adding it.

From a quick search looks like most of the currently supported drivers also have information_schema so maybe implementing it explicitly for each driver isn't that complex and won't require a 3rd party.

driver	information_schema	notes
postgres	✅
mysql	✅
sqlite3	❌	Can use sqlite_master instead
mssql	✅
redshift	✅
tidb	✅	should be MySQL compatible
clickhouse	✅	https://clickhouse.com/docs/en/operations/system-tables/information_schema/
vertica	❌	Can use v_catalog instead

[mfridman]

Correct, this would be implemented for each database based on what it supports. If we have a database-native way of asking "does this table exist" then we leverage it, otherwise, we continue doing what we do today (for that specific database).

I'm a bit hesitant to expose create table operation to users, because if goose loses control of owning that table it'll make it difficult to evolve goose, e.g., #288. I am still unsure how to do this in a backward-compatible way, but that's outside the scope of this discussion.

I think a reasonable approach might be to extend the SQLDialect interface with a versionTableExists method, which may return errNotImplemented. Then in EnsureDBVersion we call that method and check for an explicit error, if it's not implemented then we fallback to trying to create the table (like we do today), but if it is implemented we get a true|false answer, and then either create or not create the table.

[ori-shalom]

@mfridman I added another PR with the desired change.

The old PR can be closed.
I didn't manage to properly test vertica locally on my m1 silicon as it looks like there are some other issues with it.

[mfridman]

Sorry for the delay, I'll try to get to this issue (and #463) whenever I can. A bit swamped with work at the moment.

[mfridman]

Figured I'd drop by with an idea. The new Provider supports bringing your own custom Store, which currently has a limited number of methods.

However, for more advanced use cases you could extend the default Store and add a method that goose knows about. For example,

func (p *Provider) ensureVersionTable(ctx context.Context, conn *sql.Conn) (retErr error) {
	// existor is an interface that extends the Store interface with a method to check if the
	// version table exists. This API is not stable and may change in the future.
	type existor interface {
		TableExists(context.Context, database.DBTxConn, string) (bool, error)
	}
	if e, ok := p.store.(existor); 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 {

All you have to do is embed a Store and then add your own TableExists method. Wdyt?

Added an example of how this might work in fad964c

tl;dr - here's how it'd work for sqlite3, but could be extended to practically any database.

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`
	var exists bool
	if err := db.QueryRowContext(ctx, q, name).Scan(&exists); err != nil {
		return false, err
	}
	return exists, nil
}

[mfridman]

Added a mechanism to achieve this in #860, using postgres as an example

It should now be possible to do this both for existing dialects and custom ones in a standardized way.

[mfridman]

Ended up merging #860, which means we could take a lot of the logic from #463 and apply it to the rest of the databases, all it'd take is to implement a TableExists(tableName string) string on each dialect that makes sense.

E.g., here's the postges implementation

func (p *Postgres) TableExists(tableName string) string {
	schemaName, tableName := parseTableIdentifier(tableName)
	q := `SELECT EXISTS ( SELECT FROM pg_tables WHERE schemaname = '%s' AND tablename = '%s' )`
	return fmt.Sprintf(q, schemaName, tableName)
}