Field should be unique but is not, causing unnecessary ALTER TABLE executions

[siggy]

GORM Playground Link

https://github.com/go-gorm/playground/pull/525

Description

Defining a model field with the uniqueIndex tag:

type UniqueTest struct {
	UniqueIndex string `gorm:"size:191;uniqueIndex"`
}

AutoMigrate will always execute ALTER TABLE, because it does not recognize the field as unique, due to failing uniqueness check: https://github.com/go-gorm/gorm/blob/2c56954cb12dd33fc8f1875a735091d61daff702/migrator/migrator.go#L464-L470

... presumably because the field does not have a unique tag: https://github.com/go-gorm/gorm/blob/2c56954cb12dd33fc8f1875a735091d61daff702/schema/field.go#L116

Defining the model this way fixes the issue:

type UniqueTest struct {
	UniqueIndex string `gorm:"size:191;unique;uniqueIndex"`
}

This is reproducible with sqlite, mysql, and postgres:

GORM_DIALECT=sqlite GORM_ENABLE_CACHE=true ./test.sh
GORM_DIALECT=mysql GORM_ENABLE_CACHE=true ./test.sh
GORM_DIALECT=postgres GORM_ENABLE_CACHE=true ./test.sh

[rwrz]

So true! I discovered why and found a way to fix it.

They have changed how the "unique" field is defined (this line):

Unique:                 utils.CheckTruth(tagSetting["UNIQUE"]), 

So, intead of using index:,unique OR uniqueIndex, change to unique, like this:

type UniqueTest struct {
	UniqueIndex string `gorm:"size:191;unique"`
}

It will fix the problem.

[rwrz]

Issues related: #5556 , #5341 and probably more. @jinzhu researching more... seems there is a discrepancy between what migrators drivers considers as a Unique ColumnField and GORM's Fields (from the reflection).

GORM considers unique fields if it has this TAG: unique (could not find anything on the DOCs about it). Drivers considers a unique field if it has a unique constraint.

GORM considers uniqueIndex or index:,unique as an index to be created, not as a FIELD property.

Then, the migrator, checks for columnType (if it has the unique constraint) and field.unique, if it diverges, it will create a unique constraint. It doesn't make sense, since it will also check if the index exists. Well, I'm not a 100% sure, but I'm trying to help in the investigation. At least we have a workaround now (if you don't need custom index names).

[a631807682]

It seems that we only dealt with it in the index without changing the Unique property of the Field. https://github.com/go-gorm/gorm/blob/master/schema/index.go#L34 https://github.com/go-gorm/gorm/blob/master/schema/field.go#L116

I think the Unique property of Field should be like this

-               Unique:                 utils.CheckTruth(tagSetting["UNIQUE"]),
+               Unique:                 utils.CheckTruth(tagSetting["UNIQUEINDEX"]) || utils.CheckTruth(tagSetting["UNIQUE"]),

I haven't tested it, I'm not sure if it makes a breaking change to the drivers.

@rwrz Are you interested in creating a PR for it? This will help us improve support for unique and uniqueIndex in all drivers.

[rwrz]

If the UNIQUE fields are the same as having UNIQUE INDEXES, why not checking if we do have an unique index first, then if we have it, toggle field.unique == true ?

Instead of relying only on the UNIQUE|UNIQUEINDEX TAGs? My main point is if we have a unique index implemented like: index:idx_my_unique,unique, the tag checking won't be enough.