database/gdb: enhance the underlying data transformation when querying data #3557
Conversation
wln32
changed the title
|
代码量有点多，需要花点时间review。 |
The amount of code is a bit large and it will take some time. |
| @@ -1621,6 +1622,13 @@ func Test_Types(t *testing.T) { | |||
| Bit int8 | |||
| TinyInt bool | |||
| } | |||
| now, err := db.Query(ctx, "select now();") | |||
| t.Log("mysql now=", now, err) | |||
There was a problem hiding this comment.
为啥要在单测里打日志，下面也一样
这个当然是有原因的，可以看issue #3558 关于gtime的bug，等下就把他删掉
| } | ||
|
|
||
| func Test_Core_Convert_Custom(t *testing.T) { | ||
|
|
There was a problem hiding this comment.
函数开头结尾为啥要带空行，有哪个标准是这样做的吗
可以有，也可以开启golangci-lint 检测拦截这种处理
| if i > 0 { | ||
| if v.Kind() == reflect.Pointer { | ||
| if v.IsNil() { | ||
| v.Set(reflect.New(v.Type().Elem())) | ||
| } | ||
| v = v.Elem() | ||
| } | ||
| } | ||
| v = v.Field(x) |
| if len(c.StructFieldIndex) == 1 { | ||
| return structValue.Field(c.StructFieldIndex[0]) | ||
| } | ||
| v := structValue |
There was a problem hiding this comment.
为啥用变量再赋值一次
这里当时偷懒了，因为这个方法的逻辑是从标准库cv过来的
database/gdb/gdb_model_select.go
Outdated
| if ok { | ||
| // It needs to be deleted, otherwise it will cause | ||
| // conflicts as long as the struct name is the same within different functions during testing | ||
| defer tablesMap.Delete(tableName) | ||
|
|
||
| table = tableValue.(*Table) | ||
| structPointerValue := reflect.ValueOf(pointer).Elem() | ||
|
|
||
| var structValue = reflect.New(elemType) | ||
| // UnmarshalValue | ||
| fn, ok := structValue.Interface().(iUnmarshalValue) | ||
| if ok { | ||
| err = fn.UnmarshalValue(one) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| structValue = structValue.Elem() | ||
| } else { | ||
| structValue = structValue.Elem() | ||
| for _, field := range table.fields { | ||
| fieldValue := field.GetReflectValue(structValue) | ||
| value := one[field.ColumnFieldName] | ||
| if value == nil { | ||
| continue | ||
| } | ||
| fieldValue.Set(reflect.ValueOf(value.Val())) | ||
| } | ||
| } | ||
| if elemIsPtr { | ||
| structValue = structValue.Addr() | ||
| } | ||
| structPointerValue.Set(structValue) | ||
| } else { | ||
| if err = one.Struct(pointer); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
@oldme-git 这里不能通过提前判断!ok 来返回，因为不管ok为true或者是false，都需要调用model.doWithScanStruct，来判断是否需要关联查询
There was a problem hiding this comment.
在 !ok 中提前返回不可以吗
不行，因为最后需要检查是否需要with关联查询，不过我已经把这个逻辑单独提取出来一个方法了，这样就可以提前返回了
添加注释ok |
Add comment ok |
|
@oldme-git 原来的类型转换依赖于gconv，gconv的转换过于复杂，从数据库的数据到结构体，不需要那么复杂的转换。此次pr的代码看起来可能比较多，其实就两部分，一部分是解析结构体字段，拿到字段的tag，索引，字段类型之类的，缓存起来，还有一部分就是具体的转换函数，这部分是最多的，但也是逻辑最简单的，就是挨个判断类型，然后匹配赋值即可 |
@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. |
|
改动确实有点多，需要多花些时间来 |
There are indeed a lot of changes, so it will take more time to |
gqcn
added
the
slow reviewing
本次改动只修改了Scan方法的底层的类型转换实现，会走新的逻辑，依旧是先把数据转换到map，然后在转到结构体上，
和之前的行为保持一致，不会导致不兼容。
从map到结构体的转换和之前不同的是，由于在底层转换时已经转成了具体类型，不需要调用gconv来做转换，可以直接使用循环赋值
未修改之前的
修改后的
性能方面大幅提升，如果是直接一步赋值到结构体上，只需要600ms，但这样会导致有些api不兼容
一些建议
和orm的转换有关的接口，应当实现Scan和Value这两个接口来做，和标准库保持一致
建议废弃掉All，One之类的方法，一点都不好用，如果查询的是单个字段，或者是一个[]int，还需要再次转换才能使用，不如直接把Scan方法加强，支持 int , []int 这种类型的参数，一步赋值到目标字段，而不是走中间变量来回重复的转换
关于HookFuncSelect 的Next方法的返回值，不应该把原始的数据结构暴漏出去让用户操作，应当封装到结构体内部，通过方法去操作