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 purego driver #93

Closed
wants to merge 5 commits into from
Closed

Add purego driver #93

wants to merge 5 commits into from

Conversation

Elara6331
Copy link

@Elara6331 Elara6331 commented Apr 29, 2022

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

This PR adds a purego driver that can be built via tags. The test for purego doesn't use a custom function on the custom driver as it is currently impossible to do that. It instead registers the function on the default driver. The cgo test is unchanged. Other than that, everything should work the same, just pass -tags purego when building or testing to switch to the purego driver. Building without the flag will build using the cgo driver.

If anything should be changed, I would be happy to do so.

#77

User Case Description

My use case for this change is easy cross-compilation and portability of binaries using GORM.

@glebarez
Copy link
Contributor

glebarez commented May 11, 2022

@Arsen6331, to pass as GORM compatible driver, this should also succeed in all tests of GORM itself (see https://github.com/go-gorm/gorm/tree/master/tests).
You can trigger those tests via Github actions, just make a PR to https://github.com/go-gorm/playground, making changes in go.mod, go.sum and db.go to point the tests to your sqlite driver.

Hope this helps to clear things out.

@Elara6331
Copy link
Author

How would I pass a tag to github actions? You need to pass the -tags purego flag to go build, go test, etc. to use the new driver.

@Elara6331
Copy link
Author

I ran the tests on my desktop. It passed, here are the results:

$ GORM_DIALECT=sqlite CGO_ENABLED=0 go test -tags purego
2022/05/11 16:29:23 testing sqlite3...

2022/05/11 16:29:23 /home/arsen/Workspace/playground/main_test.go:14
[0.214ms] [rows:1] INSERT INTO `users` (`created_at`,`updated_at`,`deleted_at`,`name`,`age`,`birthday`,`company_id`,`manager_id`,`active`) VALUES ("2022-05-11 16:29:23.801","2022-05-11 16:29:23.801",NULL,"jinzhu",0,NULL,NULL,NULL,false) RETURNING `id`

2022/05/11 16:29:23 /home/arsen/Workspace/playground/main_test.go:17
[0.073ms] [rows:1] SELECT * FROM `users` WHERE `users`.`id` = 1 AND `users`.`deleted_at` IS NULL ORDER BY `users`.`id` LIMIT 1
PASS
ok  	gorm.io/playground	0.015s

@Elara6331
Copy link
Author

I've figured out how to do the tests in the playground repo: go-gorm/playground#476

@glebarez
Copy link
Contributor

glebarez commented May 12, 2022

I've figured out how to do the tests in the playground repo: go-gorm/playground#476

I think GORM-playground unfortunately doesn't take into account the modified go.mod file, since inside Github actions, the fresh clone of GORM is happening, and the inner tests still use the GORM's original go.mod.

I suggest you try cloning GORM itself, and change go.mod inside tests folder, then run tests from there. (just don't forget to disable CGo, just to be sure that your driver is used)

See this PR for example of such driver switch: https://github.com/go-gorm/gorm/pull/4921/files

@Elara6331
Copy link
Author

I just tried disabling CGo, and the test failed because -race needs CGo according to the error message. I removed -race and the test succeeded, so it is in fact using the driver I added.

@jinzhu
Copy link
Member

jinzhu commented May 17, 2022

sounds a reasonable solution for me, but seems like the gorm tests failed even w/o the race tag

@Elara6331
Copy link
Author

sounds a reasonable solution for me, but seems like the gorm tests failed even w/o the race tag

The latest test is successful, with cgo disabled.

@jinzhu
Copy link
Member

jinzhu commented May 17, 2022

GORM_DIALECT=sqlite CGO_ENABLED=0 go test -tags purego still fail for me for those tests. https://github.com/go-gorm/gorm/tree/master/tests

@Elara6331
Copy link
Author

GORM_DIALECT=sqlite CGO_ENABLED=0 go test -tags purego still fail for me for those tests. https://github.com/go-gorm/gorm/tree/master/tests

I just tried, they do seem to fail. Interestingly, https://github.com/glebarez/sqlite imports https://github.com/glebarez/go-sqlite, which uses the same driver as I do here (modernc.org/sqlite) but doesn't fail the tests. I'm not sure what the difference is between them.

@glebarez
Copy link
Contributor

the https://github.com/glebarez/go-sqlite is a slightly different frontend (database/sql implementation) for modernc.org/sqlite.
Thus it behaves more like https://github.com/mattn/go-sqlite3, but uses pure-go sqlite under the hood.
That's the reason it succeeds in GORM tests. It's not just a simple dependency switch.

@Elara6331
Copy link
Author

Ok then, I'll modify the PR to use glebarez/go-sqlite if you don't mind.

@glebarez
Copy link
Contributor

Ok then, I'll modify the PR to use glebarez/go-sqlite if you don't mind.

Well, it's still not going to work as a simple tag switch for existing users of this driver, because modernc.org/sqlite (and https://github.com/glebarez/go-sqlite) still uses different syntax for PRAGMA specification in connection strings (e.g. enabling foreign-key constraint)

@Elara6331

This comment was marked as outdated.

@Elara6331
Copy link
Author

I just tried using https://github.com/glebarez/go-sqlite, and all the gorm tests succeeded.

@Elara6331
Copy link
Author

Oh, you meant the cgo and purego drivers use different connection string syntax. It's 2 AM here and I'm tired, sorry for not realizing that.

@Elara6331
Copy link
Author

It looks like it wouldn't be too difficult to convert between the two syntaxes, if that's a suitable solution. A better solution might just be to document this in the README and warn users about the difference.

jinzhu added a commit that referenced this pull request Oct 9, 2022
@jinzhu jinzhu closed this Oct 9, 2022
aymanbagabas added a commit to aymanbagabas/sqlite that referenced this pull request Jul 5, 2023
This implements a pure-go alternative driver whenever CGO_ENABLED=0

Related: go-gorm#93
Fixes: go-gorm#35
Fixes: go-gorm#72
Fixes: go-gorm#77
Fixes: go-gorm#160
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