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

AutoMigrate runs CREATE TABLE multiple times #5472

Closed
brimstone opened this issue Jun 30, 2022 · 2 comments · Fixed by go-gorm/sqlite#105
Closed

AutoMigrate runs CREATE TABLE multiple times #5472

brimstone opened this issue Jun 30, 2022 · 2 comments · Fixed by go-gorm/sqlite#105
Assignees
Labels
type:feature_request feature request type:with reproduction steps with reproduction steps

Comments

@brimstone
Copy link

GORM Playground Link

go-gorm/playground#487

Description

When there's no existing table, everything is fine.

When a table exists already, the migrator wants to delete it and recreate it about as many times as there are columns. This shouldn't happen.

I'm running test.sh as:

GORM_DIALECT=sqlite DEBUG=true ./test.sh

My use case is sqlite, so I haven't seen this with mysql or postgres, but ./test.sh seems to not see it with mysql or postgres either.

Am I maybe holding gorm wrong?

@github-actions github-actions bot added the type:with reproduction steps with reproduction steps label Jun 30, 2022
@a631807682
Copy link
Member

Sqlite driver patch column by parse ddl and recreate table when column string not equal.
This is because we only have the way to parse sql activity column information, and not contain all the information, we also need to parse the index or other information by ourselves, which makes the patch process very complicated.

After SQLite version 3.16.0 (2017-01-02), we can use https://www.sqlite.org/pragma.html#pragfunc which can be the same with other drivers.

https://stackoverflow.com/questions/947215/how-to-get-a-list-of-column-names-on-sqlite3-database

@a631807682 a631807682 added the type:feature_request feature request label Jun 30, 2022
@jeffgreenca
Copy link

jeffgreenca commented Jul 5, 2022

I believe this is a bug that was introduced with #5359 the default value and not null checks specifically.

I can reproduce with the below code and a pinned go module to the commit prior to that PR:

# PR 5359 merge commit, table columns are migrated every time
go get gorm.io/gorm@93986de8e43bc9af6864621c9a4855f0f860cde2
# immediately prior commit, issue does not exist
go get gorm.io/gorm@dc1ae394f329340

Code to reproduce:

// main.go
package main

import (
	"gorm.io/driver/sqlite"
	"gorm.io/gorm"
)

type MyModel struct {
	*gorm.Model
	foo string
}

func main() {
	db, err := gorm.Open(sqlite.Open("example.db"), &gorm.Config{})
	if err != nil {
		panic(err)
	}

	db.Debug().AutoMigrate(&MyModel{})
}

My debugging shows this first clause evaluates true for the non-primary fields in the model above, specifically the change from checking default value against field.DefaultValue to instead checking default value against DefaultValueInterface.

// gorm/migrator/migrator.go around line 450
// for `created_at` column
// dv == ""
// dvNotNull == true
// field.DefaultValueInterface == nil
		dv, dvNotNull := columnType.DefaultValue()
		if dvNotNull && field.DefaultValueInterface == nil {
			// defalut value -> null
			alterColumn = true

Previously it was

// v == "" == dv
// ok == true == dvNotNull
// field.DefaultValue == ""
	if v, ok := columnType.DefaultValue(); ok && v != field.DefaultValue {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature_request feature request type:with reproduction steps with reproduction steps
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants