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.

Sign up for GitHub

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

Jump to bottom

sqlite 驱动从 github.com/jinzhu/gorm/dialects/sqlite 改为 github.com/glebarez/go-sqlite #1626

Merged
merged 3 commits into from Feb 8, 2023
Merged

sqlite 驱动从 github.com/jinzhu/gorm/dialects/sqlite 改为 github.com/glebarez/go-sqlite #1626

merged 3 commits into from Feb 8, 2023

Conversation

VigorFox
Copy link
Contributor

@VigorFox VigorFox commented Feb 6, 2023

sqlite 驱动从 github.com/jinzhu/gorm/dialects/sqlite 改为 github.com/glebarez/go-sqlite，以移除对 cgo 的依赖

TODO: 之前的 github.com/jinzhu/gorm 项目已经移至 github.com/go-gorm/gorm，并支持更多的特性

@CLAassistant
Copy link

CLAassistant commented Feb 6, 2023 •
edited

CLA assistant check
All committers have signed the CLA.

@HFO4
Copy link
Member

HFO4 commented Feb 6, 2023 •
edited

这个变更挺大的，需要仔细 SQLite 下各项功能是否还正常工作。我之前也有留意到这个纯 go 的实现，但是似乎使用人数不是太高，所以一直没敢迁移。

HFO4
HFO4 reviewed Feb 6, 2023
View reviewed changes
models/init.go Show resolved Hide resolved
@mritd
Copy link
Contributor

mritd commented Feb 7, 2023

刚想说问问要不要迁移, 现在 CGO 实现的对于交叉编译太不友好了.

@mritd mritd mentioned this pull request Feb 7, 2023
Merged
HFO4
HFO4 previously approved these changes Feb 7, 2023
View reviewed changes
models/dialect_sqlite.go
@@ -0,0 +1,288 @@
package model
Copy link
Member

Choose a reason for hiding this comment

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

没看懂为啥要包含这个文件，glebarez/go-sqlite 不是已经针对 gorm 做了兼容吗？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

glebarez/go-sqlite 注册的驱动名是 "sqlite", mattn/go-sqlite3 注册的驱动名是 "sqlite3", jinzhu/gorm/dialect_sqlite3.go 里只写了 "sqlite3" 驱动名的支持；要支持 "sqlite" 这个驱动名，除了新加一个 dialect_sqlite，也没找到更优雅的方式做这个事情。go-gorm/gorm 已经对驱动依赖进行了剥离，如果条件允许我是推荐迁移到 go-gorm/gorm（工作量不小）

models/init.go Show resolved Hide resolved
@HFO4 HFO4 dismissed their stale review February 7, 2023 11:29

incorrect

HFO4
HFO4 reviewed Feb 7, 2023
View reviewed changes
models/init.go Outdated Show resolved Hide resolved
@HFO4 HFO4 merged commit 9c58278 into cloudreve:master Feb 8, 2023
@HFO4
Copy link
Member

HFO4 commented Feb 8, 2023

Thanks! This will be a huge improvement for cross-compiling!

@HFO4
Copy link
Member

HFO4 commented Feb 12, 2023

I need to revert this change because DB auto migration is broken.
HasTable or HasColumn in SQLite dialect is not working under the new SQLite driver.

@HFO4
Copy link
Member

HFO4 commented Feb 12, 2023

I need to revert this change because DB auto migration is broken. HasTable or HasColumn in SQLite dialect is not working under the new SQLite driver.

Fixed in 37cb292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mritd mritd mritd left review comments

@HFO4 HFO4 HFO4 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@VigorFox @CLAassistant @HFO4 @mritd