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

Add mock versions of SQLiteDriver and SQLiteConn for +build !cgo #819

Merged
merged 1 commit into from Jun 5, 2020
Merged

Add mock versions of SQLiteDriver and SQLiteConn for +build !cgo #819

merged 1 commit into from Jun 5, 2020

Conversation

arp242
Copy link
Contributor

@arp242 arp242 commented Jun 5, 2020

My app can use PostgreSQL and – optionally – SQLite. I would like to be
able to compile the app without cgo when SQLite isn't used, as this
removes the need for a C compiler which makes builds easier and faster,
especially for end-users.

In the simple case, this is not a problem go-sqlite3 already provides a
simple non-cgo mock so it compiles and gives a runtime error if you try
to use it anyway.

However, now I'd like to register a function for my SQLite connection to
match a PostgreSQL function like so:

sql.Register("sqlite3_custom", &sqlite3.SQLiteDriver{
	ConnectHook: func(conn *sqlite3.SQLiteConn) error {
		return conn.RegisterFunc("pow", pow, true); err != nil {
	},
})

But this makes it quite hard to keep the same logic since it refers to
types that don't exist with CGO_ENABLED=0. I will need to create a db.go
with +build !cgo and db_cgo.go with +buid cgo which duplicates all
the logic but with the sqlite hooks. In my case, this actually affects
quite a lot; for example I have a helper function which connects and
runs migrations and whatnot which looks like:

type ConnectOptions struct {
	Connect    string // Connect string.
	Schema     []byte // Database schema to create on startup.
	Migrate    *Migrate
	SQLiteHook func(*sqlite3.SQLiteConn) error
}

And I'd have to have two versions of that, too. You could perhaps do
something with interfaces, but because the sql.Register() call above
references the sqlite3.SQLiteDriver.ConnectHook struct field that's
not so straightforward (and wrapping stuff in interfaces probably won't
do much for the general clarity either).

This simplifies all of that by providing some common types that may be
used when setting up a SQLite connectin. I renamed the
SQLiteDriverMock to &SQLiteDriver for this reason. As far as I can
tell in my testing, this has no real downsides (but perhaps I missed
something?)


Note: it might also be worth doing something similar for error.go, as I
already have two variants of the below function (one with cgo as below,
and one without cgo which checks just PostgreSQL):

// ErrUnique reports if this error reports a UNIQUE constraint violation.
//
// This is the cgo version which works for PostgreSQL and SQLite.
func ErrUnique(err error) bool {
	var sqlErr *sqlite3.Error
	if errors.As(err, &sqlErr) && sqlErr.ExtendedCode == sqlite3.ErrConstraintUnique {
		return true
	}
	var pqErr *pq.Error
	if errors.As(err, &pqErr) && pqErr.Code == "23505" {
		return true
	}
	return false
}

This is a lot more manageable than the ConnectHook case, but it would be
nicer if it would work without the need for build tags.

My app can use PostgreSQL and – optionally – SQLite. I would like to be
able to compile the app without cgo when SQLite isn't used, as this
removes the need for a C compiler which makes builds easier and faster,
especially for end-users.

In the simple case, this is not a problem go-sqlite3 already provides a
simple non-cgo mock so it compiles and gives a runtime error if you try
to use it anyway.

However, now I'd like to register a function for my SQLite connection to
match a PostgreSQL function like so:

	sql.Register("sqlite3_custom", &sqlite3.SQLiteDriver{
		ConnectHook: func(conn *sqlite3.SQLiteConn) error {
			return conn.RegisterFunc("pow", pow, true); err != nil {
		},
	})

But this makes it quite hard to keep the same logic since it refers to
types that don't exist with CGO_ENABLED=0. I will need to create a db.go
with `+build !cgo` and db_cgo.go with `+buid cgo` which duplicates all
the logic but with the sqlite hooks. In my case, this actually affects
quite a lot; for example I have a helper function which connects and
runs migrations and whatnot which looks like:

	type ConnectOptions struct {
		Connect    string // Connect string.
		Schema     []byte // Database schema to create on startup.
		Migrate    *Migrate
		SQLiteHook func(*sqlite3.SQLiteConn) error
	}

And I'd have to have two versions of that, too. You could perhaps do
something with interfaces, but because the sql.Register() call above
references the `sqlite3.SQLiteDriver.ConnectHook` struct field that's
not so straightforward (and wrapping stuff in interfaces probably won't
do much for the general clarity either).

This simplifies all of that by providing some common types that may be
used when setting up a SQLite connectin. I renamed the
`SQLiteDriverMock` to `&SQLiteDriver` for this reason. As far as I can
tell in my testing, this has no real downsides (but perhaps I missed
something?)

---

Note: it might also be worth doing something similar for error.go, as I
already have two variants of the below function (one with cgo as below,
and one without cgo which checks just PostgreSQL):

	// ErrUnique reports if this error reports a UNIQUE constraint violation.
	//
	// This is the cgo version which works for PostgreSQL and SQLite.
	func ErrUnique(err error) bool {
		var sqlErr *sqlite3.Error
		if errors.As(err, &sqlErr) && sqlErr.ExtendedCode == sqlite3.ErrConstraintUnique {
			return true
		}
		var pqErr *pq.Error
		if errors.As(err, &pqErr) && pqErr.Code == "23505" {
			return true
		}
		return false
	}

This is a lot more manageable than the ConnectHook case, but it would be
nicer if it would work without the need for build tags.
@mattn
Copy link
Owner

mattn commented Jun 5, 2020

Sounds good to me. I'll look it in later.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 53.404% when pulling 678cb5b on zgoat:cgo into f600c4b on mattn:master.

@mattn mattn merged commit 0cec2d7 into mattn:master Jun 5, 2020
@mattn
Copy link
Owner

mattn commented Jun 5, 2020

Thank you!

@arp242 arp242 deleted the cgo branch June 5, 2020 13:12
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.

None yet

3 participants