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

feat(database/gdb): add time field type for value converting for/from field #3712

Merged
merged 10 commits into from
Sep 10, 2024
Merged

feat(database/gdb): add time field type for value converting for/from field #3712

merged 10 commits into from
Sep 10, 2024

Conversation

cyjaysong
Copy link
Contributor

Convert Time based on fieldType

gqcn
gqcn requested changes Jul 29, 2024
View reviewed changes
os/gtime/gtime_sql.go Outdated Show resolved Hide resolved
database/gdb/gdb_types.go Outdated Show resolved Hide resolved
@gqcn gqcn changed the title database/gdb: Convert Time based on fieldType feat(database/gdb): add time field type for field converting Jul 29, 2024
@gqcn gqcn changed the title feat(database/gdb): add time field type for field converting feat(database/gdb): add time field type for value converting for field Jul 29, 2024
@gqcn gqcn changed the title feat(database/gdb): add time field type for value converting for field feat(database/gdb): add time field type for value converting for/from field Jul 29, 2024
@cyjaysong
Copy link
Contributor Author

cyjaysong commented Jul 30, 2024 •
edited
Loading

@gqcn 移除gtime的driver.Valuer 接口实现，使用ConvertValueForField中的类型匹配规则进行序列化

@cyjaysong cyjaysong mentioned this pull request Jul 30, 2024
Merged
@cyjaysong cyjaysong requested a review from gqcn July 30, 2024 08:34
gqcn
gqcn requested changes Aug 1, 2024
View reviewed changes
os/gtime/gtime_format.go Outdated Show resolved Hide resolved
@wln32
Copy link
Member

wln32 commented Aug 2, 2024

@gqcn 移除的 接口实现，使用中的类型匹配规则进行序列化gtime``driver.Valuer``ConvertValueForField

我觉得没啥必要加这个接口，用Scan和Value和标准库一样，就可以

@cyjaysong
Copy link
Contributor Author

@gqcn 移除的 接口实现，使用中的类型匹配规则进行序列化 gtimedriver.ValuerConvertValueForField

我觉得没啥必要加这个接口，用Scan和Value和标准库一样，就可以

没加接口了啊，新加的接口移除掉了

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

@gqcn removed the interface implementation, use the type matching rules in serialization gtime driver.ValuerConvertValueForField

I don’t think there is any need to add this interface. Just use Scan and Value the same as the standard library.

No interfaces are added. The newly added interfaces have been removed.

@niluan304
Copy link
Contributor

我觉得不应该移除 gtime.Time 的 driver.Valuer 接口实现，
如果项目使用的是其他 orm，如 gorm，而字段类型又是 gtime.Time，可能会导致无法序列化为数据库中对应的类型。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

I think the driver.Valuer interface implementation of gtime.Time should not be removed,
If the project uses other ORM, such as gorm, and the field type is gtime.Time, it may not be serialized to the corresponding type in the database.

gqcn
gqcn requested changes Aug 14, 2024
View reviewed changes
os/gtime/gtime_sql.go Outdated Show resolved Hide resolved
@cyjaysong
Copy link
Contributor Author

@gqcn 这个ci失败看着不像我这边的问题，我没改gview

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

@gqcn This CI failure doesn’t look like a problem on my side. I didn’t change gview.

@niluan304
Copy link
Contributor

@gqcn 这个ci失败看着不像我这边的问题，我没改gview

可能是github服务故障导致的：

https://www.githubstatus.com/incidents/kz4khcgdsfdv

试试重新运行一下 action

@cyjaysong
Copy link
Contributor Author

@niluan304 怎么重新Action

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Aug 19, 2024

Quality Gate Passed Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.5% Duplication on New Code

See analysis details on SonarCloud

@cyjaysong
Copy link
Contributor Author

@gqcn 不看看了吗

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

@gqcn Don’t you want to take a look?

@gqcn gqcn merged commit bb9a3b8 into gogf:master Sep 10, 2024
24 checks passed
wln32 added a commit to wln32/gf that referenced this pull request Sep 12, 2024
@gqcn @cyjaysong
@niluan304 @wln32
fix/3764 (#4)
247243c 
* feat(database/gdb): add `time` field type for value converting for/from field (gogf#3712)

* fix(net/goai): fix openapi miss `required` tag of `BizRequest` when set `CommonRequest` (gogf#3724)

* perf(database/gdb): performance improvement for struct scanning when with feature disabled (gogf#3677)

* up

---------

Co-authored-by: CyJaySong <29367599+cyjaysong@users.noreply.github.com>
Co-authored-by: Zwei <zwei.elen@outlook.com>
Co-authored-by: wln32 <49137144+wln32@users.noreply.github.com>
@cyjaysong cyjaysong deleted the gtime branch September 20, 2024 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gqcn gqcn gqcn 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.
5 participants
@cyjaysong @wln32 @Issues-translate-bot @niluan304 @gqcn