Skip to content
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

Extend Adapter interface to include function to return adapter name #345

Open
lafriks opened this issue Sep 28, 2023 · 6 comments
Open

Extend Adapter interface to include function to return adapter name #345

lafriks opened this issue Sep 28, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@lafriks
Copy link
Contributor

lafriks commented Sep 28, 2023

Would be useful to allow check what adapter is used currently so that database type specific code could be executed only on right adapters.

func Name() string

or

func Type() string

that would return mysql, postgres, sqlite3 etc

@lafriks lafriks added the enhancement New feature or request label Sep 28, 2023
@Fs02
Copy link
Member

Fs02 commented Sep 28, 2023

could you give examples?

and is the database type specific code will be in sql package?

@lafriks
Copy link
Contributor Author

lafriks commented Sep 29, 2023

no it's for usage in user code later on, for one I would want this to use in migrator package for such code to be able to lock down migrations so that if multiple instances are started at same time (for example in containers) they would not run same migrations:

	if m.typ == DatabaseSQLite3 {
		if err := m.sync(ctx); err != nil {
			cancelUpdate(err)
		}
		return func(ctx context.Context, vers int64) {
			if vers == 0 {
				return
			}
			if err := m.repo.Insert(ctx, &version{Version: vers}); err != nil {
				cancelUpdate(err)
			}
		}, func() {}
	}

	updZeroVersion := "INSERT INTO versions(version, created_at) VALUES (0, current_timestamp) ON CONFLICT(version) DO UPDATE SET updated_at = current_timestamp"
	if m.typ == DatabaseMySQL {
		updZeroVersion = "INSERT INTO versions(version, created_at) VALUES (0, current_timestamp) ON DUPLICATE KEY UPDATE updated_at = current_timestamp"
	}

currently because of that I have to copy over migrator code and can not really implement it upstream as instead of go-rel/migration package: https://github.com/go-rel/migration/blob/dbef2632dc4efecbb107b4c61648a0d0d255c0ac/migration.go#L170
I use:

// New migratorr.
func New(typ string, repo rel.Repository) *Migrator {
	return &Migrator{
		typ:  typ,
		repo: repo,
	}
}

instead I could use for db specific code something like m.repo.Adapter().Type() == "sqlite3" instead of having to keep around current database type in code and pass it all around for database type specific SQLs etc

@Fs02
Copy link
Member

Fs02 commented Oct 1, 2023

got it, I think it's should be okay to add some metadata information like this

I prefer func Name() string, and maybe have a constant (ex: Name) in each adapter package so comparison can be done like: m.repo.Adapter().Type() == sqlite3.Name

@lafriks
Copy link
Contributor Author

lafriks commented Oct 1, 2023

Ok, I will create PR

@lafriks
Copy link
Contributor Author

lafriks commented Oct 3, 2023

@Fs02 please review:
#346
go-rel/sql#59
go-rel/sqlite3#53

If naming/implementation looks good I will create PRs for other adapters

@Fs02
Copy link
Member

Fs02 commented Oct 14, 2023

@lafriks need to be updated too
https://github.com/go-rel/reltest/blob/main/nop_adapter.go
https://github.com/go-rel/mssql

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants