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

database/gdb: enhance the underlying data transformation when querying data #3557

Closed
wants to merge 44 commits into from
Closed

database/gdb: enhance the underlying data transformation when querying data #3557

wants to merge 44 commits into from

Conversation

wln32
Copy link
Member

@wln32 wln32 commented May 1, 2024 •
edited
Loading

本次改动只修改了Scan方法的底层的类型转换实现，会走新的逻辑，依旧是先把数据转换到map，然后在转到结构体上，

和之前的行为保持一致，不会导致不兼容。
从map到结构体的转换和之前不同的是，由于在底层转换时已经转成了具体类型，不需要调用gconv来做转换，可以直接使用循环赋值

====更新7.5
对于本次更新增加一个开关，打开时走新的逻辑，关闭时还是走旧逻辑
新增加自定义字段转换的特性，方便用户平稳过渡
1.RegisterDatabaseConvertFunc 为某个驱动(比如mysql)的某个类型(比如decimal)注册自定义转换函数
2.RegisterGoTypeConvertFunc为go语言的类型添加自定义转换函数，且优先级比1高

未修改之前的

1

修改后的

3333

性能方面大幅提升，如果是直接一步赋值到结构体上，只需要600ms，但这样会导致有些api不兼容

一些建议

  1. 不建议对一整个结构体实现UnmarshalValue接口来做赋值，例如

3

  1. 和orm的转换有关的接口，应当实现Scan和Value这两个接口来做，和标准库保持一致

  2. 建议废弃掉All，One之类的方法，一点都不好用，如果查询的是单个字段，或者是一个[]int，还需要再次转换才能使用，不如直接把Scan方法加强，支持 int , []int 这种类型的参数，一步赋值到目标字段，而不是走中间变量来回重复的转换

  3. 关于HookFuncSelect 的Next方法的返回值，不应该把原始的数据结构暴漏出去让用户操作，应当封装到结构体内部，通过方法去操作

@wln32 wln32 linked an issue May 1, 2024 that may be closed by this pull request
Some ideas on improving the Scan method of orm #3538
Closed
@wln32 wln32 requested review from gqcn and hailaz and removed request for gqcn May 1, 2024 08:06
@wln32 wln32 linked an issue May 1, 2024 that may be closed by this pull request
Data mapping to struct is slow when the number of database queries is large. #3406
Closed
@wln32 wln32 changed the title database/gdb: Improve/gdb scan typeconvert database/gdb: enhance the underlying data transformation when querying data May 1, 2024
@wln32
Copy link
Member Author

wln32 commented May 22, 2024

补充一下公共方法的注释

添加注释ok

@Issues-translate-bot
Copy link

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

Add comments about public methods

Add comment ok

@wln32
Copy link
Member Author

wln32 commented May 22, 2024

@gqcn @wln32 需要在 gdb 里面引入复杂的转换逻辑吗？这边我不是很了解

@oldme-git 原来的类型转换依赖于gconv，gconv的转换过于复杂，从数据库的数据到结构体，不需要那么复杂的转换。此次pr的代码看起来可能比较多，其实就两部分，一部分是解析结构体字段，拿到字段的tag，索引，字段类型之类的，缓存起来，还有一部分就是具体的转换函数，这部分是最多的，但也是逻辑最简单的，就是挨个判断类型，然后匹配赋值即可

@Issues-translate-bot
Copy link

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

@gqcn @wln32 Do you need to introduce complex conversion logic into gdb? I don't know much about this

@oldme-git The original type conversion relies on gconv. The conversion of gconv is too complicated. From database data to structure, there is no need for such complicated conversion. The PR code this time may seem like a lot, but it actually consists of two parts. One part is to parse the structure fields, get the field tags, indexes, field types, etc., and cache them, and the other part is the specific conversion function. This It has the most parts, but it is also the simplest in logic. It is to judge the types one by one, and then match and assign values.

@gqcn
Copy link
Member

gqcn commented May 23, 2024

改动确实有点多，需要多花些时间来review，估计会挂很长一段时间。

@Issues-translate-bot
Copy link

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

There are indeed a lot of changes, so it will take more time to review, and it will probably hang for a long time.

@gqcn gqcn added the slow reviewing It might be complicated or too many changes, which needs more time reviewing. label May 23, 2024
wln32 added 6 commits July 5, 2024 15:26
@wln32
1.增加两个可以自定义注册的字段转换函数，根据不同场景使用
3d36959 
 - RegisterDatabaseConvertFunc 注册某个驱动(比如mysql)的数据库字段类型
 - RegisterStructFieldConvertFunc 注册某个go语言结构体的字段
2. 一些细节优化
@wln32
使用SetTestEnvironment函数来控制是否删除缓存的结构体信息
c474c98
@wln32
添加自定义转换的测试
1f4e061
@wln32
ci
c8ccb49
@wln32
修复测试
342dd6f
@wln32
1.注册go结构体字段的转换变更为注册go类型的字段转换
f52440e 
2.更新自定义转换的测试逻辑
@wln32 wln32 closed this Oct 16, 2024
@cyjaysong
Copy link
Contributor

@wln32 怎么关闭了

@Issues-translate-bot
Copy link

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

@wln32 Why is it closed?

@wln32
Copy link
Member Author

wln32 commented Oct 18, 2024

@wln32 怎么关闭了

这个目前看来不会被合并了，所以没什么用了

@Issues-translate-bot
Copy link

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

@wln32 Why is it closed?

It seems that this will not be merged at present, so it is of no use.

@cyjaysong
Copy link
Contributor

@wln32怎么关闭了

这个目前看来不会被合并了，所以没什么用了

@gqcn 老郭不是说只是审核慢么

@Issues-translate-bot
Copy link

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

@wln32 why is it closed?

It seems that this will not be merged at present, so it is of no use.

@gqcn Didn’t Lao Guo say it’s just that the review is slow?

@gqcn
Copy link
Member

gqcn commented Oct 19, 2024

@wln32 @cyjaysong 只是review慢，但是代码改动优点多我每次回过头来看都很头疼。碎片时间多，每一次回头来看都是全新的开始。

@Issues-translate-bot
Copy link

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

@wln32 @cyjaysong It’s just that the review is slow, but the code changes have so many advantages that I get a headache every time I look back. There is so much fragmented time, and every time you look back it is a brand new beginning.

@wln32
Copy link
Member Author

wln32 commented Oct 19, 2024

@wln32 @cyjaysong 只是review慢，但是代码改动优点多我每次回过头来看都很头疼。碎片时间多，每一次回头来看都是全新的开始。

其实有个开关可以打开或关闭新逻辑，有bug时让用户自行关闭，走旧逻辑就可以了，然后修复bug，过度几个版本就差不多了

@Issues-translate-bot
Copy link

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

@wln32 @cyjaysong It’s just that the review is slow, but the code changes have so many advantages that I get a headache every time I look back. There is so much fragmented time, and every time you look back it is a brand new beginning.

In fact, there is a switch that can turn on or off the new logic. If there is a bug, let the user turn it off by themselves and just use the old logic. Then fix the bug and it will be almost done after a few versions.

This was referenced Jan 12, 2025
Open
Open
@cyjaysong
Copy link
Contributor

cyjaysong commented Jan 13, 2025 •
edited
Loading

@wln32
1、对于整体结构不使用接口来实现序列化和反序列化确实能省不少性能，因为大多是entity的整体结构的字段都是基本数据类型，但是对于某字段使用自定义类型的时候，用接口来实现会更合适，使用RegisterDatabaseConvertFunc和 RegisterGoTypeConvertFunc 来注册的话，自定义结构体的声明和(正/反)序列化逻辑会比较分散，如果不分散的话就需要搞一个初始化来做统一注入，相对来说比较麻烦。
2、没看太明白，我估计应该就是我说的自定义结构体实现正反序列化接口的方式，目前几个主流的orm都是使用的这种方式来实现自定义类型的正反序列化
3、All，One可以保留，方面开发者拿到数据在不同场景输出不同类型的查询结果。但是对于Scan方法的对基本数据类型和基本数据类型切片的支持，这个确实很实用
4、我目前没有场景使用With，暂时做不了建议

@Issues-translate-bot
Copy link

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

@wln32

  1. Not using interfaces to implement serialization and deserialization for the overall structure can indeed save a lot of performance, because most of the fields of the overall structure of the entity are basic data types, but when using a custom type for a certain field, use It would be more appropriate to implement the interface, using RegisterDatabaseConvertFunc and RegisterGoTypeConvertFunc If you want to register, the declaration of the custom structure and the (forward/reverse) serialization logic will be relatively scattered. If they are not scattered, you will need to do an initialization for unified injection, which is relatively troublesome.
  2. I don’t understand it very clearly. I guess it is the way I mentioned that custom structures implement forward and deserialization interfaces. Currently, several mainstream ORMs use this method to implement forward and deserialization of custom types.
  3. All and One can be retained so that developers can obtain the data and enter different types of query results in different scenarios. But for the Scan method's support for basic data types and basic data type slicing, this is indeed very practical.
  4. I currently don’t have a scenario to use With, so I can’t make any suggestions for the time being.

@wln32
Copy link
Member Author

wln32 commented Jan 13, 2025

@wln32 1、对于整体结构不使用接口来实现序列化和反序列化确实能省不少性能，因为大多是entity的整体结构的字段都是基本数据类型，但是对于某字段使用自定义类型的时候，用接口来实现会更合适，使用RegisterDatabaseConvertFunc和 RegisterGoTypeConvertFunc 来注册的话，自定义结构体的声明和(正/反)序列化逻辑会比较分散，如果不分散的话就需要搞一个初始化来做统一注入，相对来说比较麻烦。 2、没看太明白，我估计应该就是我说的自定义结构体实现正反序列化接口的方式，目前几个主流的orm都是使用的这种方式来实现自定义类型的正反序列化 3、All，One可以保留，方面开发者拿到数据在不同场景输入不同类型的查询结果。但是对于Scan方法的对基本数据类型和基本数据类型切片的支持，这个确实很实用 4、我目前没有场景使用With，暂时做不了建议

  1. 对于整体结构不使用接口来实现序列化和反序列化确实能省不少性能，因为大多是entity的整体结构的字段都是基本数据类型，但是对于某字段使用自定义类型的时候，用接口来实现会更合适，使用RegisterDatabaseConvertFunc和 RegisterGoTypeConvertFunc 来注册的话，自定义结构体的声明和(正/反)序列化逻辑会比较分散，如果不分散的话就需要搞一个初始化来做统一注入，相对来说比较麻烦。

如果类型全部是由你自己定义的，当然可以实现为接口，但是有些场景下，类型是第三方库的，那么你就不能为其实现接口了，除非你基于这个类型新定义个类型，然后实现接口，不过这样的话，如果用户升级框架，会造成一些工作量，如果使用RegisterGoTypeConvertFunc则完全没有负担，原来的类型什么的都不用改

  1. 没看太明白，我估计应该就是我说的自定义结构体实现正反序列化接口的方式，目前几个主流的orm都是使用的这种方式来实现自定义类型的正反序列化

如果要实现接口的话，我建议还是跟标准库走，实现sql.Scanner和sql.Valuer即可，不需要再定义一套新的接口
且本次更新是支持sql.Scanner接口的，由于这次更新只涉及到查询相关的，插入相关的我没看源码，不知道有没有对sql.Valuer做支持

  1. All，One可以保留，方面开发者拿到数据在不同场景输入不同类型的查询结果。但是对于Scan方法的对基本数据类型和基本数据类型切片的支持，这个确实很实用

Scan支持基础类型和基本类型切片，这个我也比较支持

@gogf gogf deleted a comment from Issues-translate-bot Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houseme houseme houseme left review comments

@oldme-git oldme-git oldme-git left review comments

@hailaz hailaz Awaiting requested review from hailaz

@gqcn gqcn Awaiting requested review from gqcn

Assignees
No one assigned
Labels
enhancement slow reviewing It might be complicated or too many changes, which needs more time reviewing.
Projects
None yet
Milestone
No milestone
6 participants
@wln32 @gqcn @Issues-translate-bot @oldme-git @cyjaysong @houseme