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

feat: AutoMigrate Automatically Generates Corresponding Table Structure Based on Model Struct Fields #3973

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Royal-go
Copy link

@Royal-go Royal-go commented Nov 26, 2024

平常写数据处理工具很多,给 goframe orm增加一个 根据结构体生成表的方法,这样减少一些繁琐的创建表操作,在 gdb 里面的model上 增加了一个方法AutoMigrate , 实现类似 gorm 根据实体 创建表的方法

@UncleChair
Copy link
Contributor

typeMapping和框架推荐的一些最佳实践不太一致,例如ORM文档中的:

我们建议使用bit(1)来表示字段的bool类型,而非tinyint(1)或者int(1)。因为tinyint(1)/int(1)字段类型表示的范围是-127~127,通常可能会被用作状态字段类型。而bit(1)的类型范围为0/1,可以很好的表示bool类型的两个值false/true

而且mapping的做法和各个数据库的耦合很大,不利于后续添加其他数据库驱动,要做gdb migration的话感觉会需要一些基于驱动的映射机制,改动会比较大。

以及GoFrame应该是不会内置migration功能的,很多issue都讨论过这个问题 #3627

要用的话不如试试引入其它的migration项目吧哈哈哈

@Issues-translate-bot
Copy link

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


typeMapping is not consistent with some best practices recommended by the framework, such as in the ORM documentation:

We recommend using bit(1) to represent the bool type of the field instead of tinyint(1) or int(1). Because the range represented by the tinyint(1)/int(1) field type is -127~127, it may usually be used as the status field type. The type range of bit(1) is 0/1, which can well represent the two values ​​false/true of the bool type.

Moreover, the mapping method is highly coupled with each database, which is not conducive to the subsequent addition of other database drivers. If you want to do gdb migration, it seems that some driver-based mapping mechanisms will be needed, and the changes will be relatively large.

And GoFrame should not have built-in migration function. Many issues have discussed this issue #3627

If you want to use it, why not try introducing other migration projects hahaha

@Royal-go
Copy link
Author

Royal-go commented Nov 29, 2024

typeMapping和框架推荐的一些最佳实践不太一致,例如ORM文档中的:

我们建议使用bit(1)来表示字段的bool类型,而非tinyint(1)或者int(1)。因为tinyint(1)/int(1)字段类型表示的范围是-127~127,通常可能会被用作状态字段类型。而bit(1)的类型范围为0/1,可以很好的表示bool类型的两个值false/true

而且mapping的做法和各个数据库的耦合很大,不利于后续添加其他数据库驱动,要做gdb migration的话感觉会需要一些基于驱动的映射机制,改动会比较大。

以及GoFrame应该是不会内置migration功能的,很多issue都讨论过这个问题 #3627

要用的话不如试试引入其它的migration项目吧哈哈哈

有用过其他的migration项目哈,只不过不想每弄一个项目都额外引入三方库

1.我的感觉最佳实践 对单一的库是可以的,每个库的类型不同,不过这里为了适配多数库,主要保持golang类型与数据库类型的映射,这一版自用 也就经常用到内存库和 mysql
2. 添加其他数据库驱动,是目前需要在这做下适配
3. gdb migration 这个 我觉得可以在后续的 驱动driver 上再增加 类型映射和自增映射两个接口方法,由具体的驱动实现,然后model的这个方法就可以去掉很多冗余了,可以解决第二个耦合问题了

@Issues-translate-bot
Copy link

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


typeMapping is not consistent with some best practices recommended by the framework, such as in the ORM documentation:

We recommend using bit(1) to represent the bool type of the field instead of tinyint(1) or int(1). Because the range represented by the tinyint(1)/int(1) field type is -127~127, it may usually be used as the status field type. The type range of bit(1) is 0/1, which can well represent the two values ​​false/true of the bool type.

Moreover, the mapping method is highly coupled with each database, which is not conducive to the subsequent addition of other database drivers. If you want to do gdb migration, it seems that some driver-based mapping mechanisms will be needed, and the changes will be relatively large.

And GoFrame should not have built-in migration function. Many issues have discussed this issue #3627

If you want to use it, why not try introducing other migration projects hahaha

I have used other migration projects, but I don’t want to introduce additional third-party libraries for every project.

  1. I feel that the best practice is applicable to a single library. Each library has different types. However, in order to adapt to most libraries, the mapping between golang types and database types is mainly maintained. This version is often used for personal use. Memory library and mysql
  2. Adding other database drivers currently requires adaptation here.
  3. For gdb migration, I think we can add two interfaces, type mapping and auto-increment mapping, to the subsequent driver. The specific driver implements the interface, and then the model method can remove a lot of redundancy.

@gqcn
Copy link
Member

gqcn commented Nov 29, 2024

@Royal-go 你好,感谢参与开源贡献!你的代码我看了,整体思路不错的。但是耦合有一点大。并且你这里只有create table功能,但一个完整的migrate特性是需要支持alter table的。同时,各个数据库对于table的创建和修改会有一些语法不一致的细节。

因此,如果需要实现一个相对完善的migration特性,可能需要在主库的ORM中增加Migrate接口,并且在各个数据库驱动中去实现Migrate方法。我个人觉得这可能是比较好的实践方式。

但是Migration特性风险最大的地方就在于alter table,可能会丢失列数据。所以在实现Migrate的时候尤其需要做严格的修改控制,一旦涉及到数据丢失的可能性,应当立即返回错误。除非用户传递允许丢失数据库的选项。

感兴趣可以尝试使用mysql驱动实现一版,我们一起来讨论。但这个特性可能需要持续的讨论,直到大家都觉得OK才会合并入主库。同时,你在实现时,可以充分利用AI协助,应该可以快速完善这块逻辑以及对应的单测。

@Issues-translate-bot
Copy link

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


Hello @Royal-go, thanks for participating in the open source contribution! I've read your code and the overall idea is good. But the coupling is a bit big. And you only have the create table function here, but a complete migrate feature needs to support alter table. At the same time, various databases will have some inconsistent syntax details for the creation and modification of table.
If you need to implement a relatively complete migration feature, you may need to add the Migrate method to the ORM of the main library and implement the Migrate method in each database driver. I personally think this may be a better practice.
But the biggest risk of the Migration feature lies in alter table, which may lose column data. Therefore, when implementing Migrate, strict modification control may be particularly important. Once the possibility of data loss is involved, an error should be returned immediately. Unless the user passes an option that allows the database to be lost.
If you are interested, you can try to use the mysql driver to implement a version, and we will discuss it together. However, this feature may require continued discussion until everyone feels it is OK before it will be merged into the main library. At the same time, you can make full use of AI assistance when implementing it, and you should be able to quickly improve this logic and the corresponding single test.

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.

4 participants