refactor: distinguish between Unique and UniqueIndex

[black-06]

• Do only one thing
• Non breaking API changes
• Tested

What did this pull request do?

See #6381

Related issues:
#6224
#5401
#5715
#5681
#6378
#6407

User Case Description

[a631807682]

Can we add more tests showing what this change fixes? The test doesn't need to be successful as I can imagine this would involve a lot of driver changes.
I take mysql as an example

1. We need to ensure that the field A marked as unique index is not a varchar(20) unique when creating table, but UNIQUE INDEX idx_a (a)
   Link AutoMigrate creating different unique Index on subsequent runs #6378
2. When comparing field A, we may need to compare field.Unique and field.UniqueIndex respectively (this may cause failure, because currently mysql does not handle the columnKey value 'MUL', and unique index may have multiple)
   Link 标签 uniqueIndex 在解析唯一性时未被判断，查看源代码发现仅判断了UNIQUE #5681
   https://github.com/go-gorm/mysql/blob/master/migrator.go#L264
3. When comparing, it is necessary to ensure the deletion and addition of the unique index
   Link Unnecessarily unique indexes created when running AutoMigrate #6224

Since this PR may contain relatively large changes, I want to describe it here. This is what I can think of so far. If this PR will fix more problems, welcome to add.

[black-06]

Can I import "github.com/stretchr/testify/assert" to "gorm.io/gorm/tests" ?
I need ElementsMatch, or copy code to tests package ?

[a631807682]

Can I import "github.com/stretchr/testify/assert" to "gorm.io/gorm/tests" ? I need ElementsMatch, or copy code to tests package ?

Both ok for me.

[black-06]

What should we do with

`gorm:"uniqueIndex;index"`

[a631807682]

What should we do with

`gorm:"uniqueIndex;index"`

This situation is meaningless, it is equivalent to gorm:"uniqueIndex;", but the following scenarios will exist

Name string `gorm:"uniqueIndex:idx_name_code"`
Code string `gorm:"uniqueIndex:idx_name_code;index"`

[black-06]

What should we do with

`gorm:"uniqueIndex;index"`

This situation is meaningless, it is equivalent to gorm:"uniqueIndex;", but the following scenarios will exist

Name string `gorm:"uniqueIndex:idx_name_code"`
Code string `gorm:"uniqueIndex:idx_name_code;index"`

Yes it's meaningless, but it parses out two IndexOption.

Or in this case:

type UserIndex struct {
	Name8        string `gorm:"index:,length:10;index:,collate:utf8"`
}

It got index

"idx_user_indices_name8": {
	Name: "idx_user_indices_name8",
	Type: "",
	Fields: []schema.IndexOption{
		{Field: &schema.Field{Name: "Name8"}, Length: 10},
		{Field: &schema.Field{Name: "Name8"}, Collate: "utf8"},
	},
}

Maybe Index.Fields ([]IndexOption) should be changed to Map[string]IndexOption which key is field name.

If changed, do we mark Fields as Deprecated and add FieldMap, or just modify?

[a631807682]

type UserIndex struct {
	Name8        string `gorm:"index:,length:10;index:,collate:utf8"`
}

The same index may indeed be created multiple times, but I am not sure whether the actual usage scenario exists. In this case, I can only think that some scenarios may need to be specified using FORCE INDEX, but in this case, I think users can specify an index name instead of using the default name (Using the default name may not be good for our comparison index).

In short, I think there are too few usage scenarios to consider this situation, of course if my thinking is wrong, please let me know.

[black-06]

In short, I think there are too few usage scenarios to consider this situation

I mean, it's an unreported bug.
We don't have to fix it (because there are very few usage scenarios).

What is your opinion? fix or not?

[a631807682]

In short, I think there are too few usage scenarios to consider this situation

I mean, it's an unreported bug. We don't have to fix it (because there are very few usage scenarios).

What is your opinion? fix or not?

We don't have to fix it, specifying the index name can avoid.

[black-06]

1. We need to ensure that the field A marked as unique index is not a varchar(20) unique when creating table, but UNIQUE INDEX idx_a (a)

For unique, we should do

CREATE TABLE `index_tests` (
  `name` VARCHAR(10),
  CONSTRAINT `uni_index_tests_name` UNIQUE (`name`)
);

or

CREATE TABLE `index_tests` (
  `name` VARCHAR(10),
  UNIQUE INDEX `idx_index_tests_name` (`name`)
);

There is no difference between them in mysql,
but I don't know the situation of other databases.

For option 1, I should add a ParseUniqueConstraints like ParseCheckConstraints.
For option 2, I should modify in ParseIndexes.

[a631807682]

1. We need to ensure that the field A marked as unique index is not a varchar(20) unique when creating table, but UNIQUE INDEX idx_a (a)

For unique, we should do

CREATE TABLE `index_tests` (
  `name` VARCHAR(10),
  CONSTRAINT `uni_index_tests_name` UNIQUE (`name`)
);

or

CREATE TABLE `index_tests` (
  `name` VARCHAR(10),
  UNIQUE INDEX `idx_index_tests_name` (`name`)
);

There is no difference between them in mysql, but I don't know the situation of other databases.

For option 1, I should add a ParseUniqueConstraints like ParseCheckConstraints. For option 2, I should modify in ParseIndexes.

Consistent with the current, we need to remove the redundant unique statement.

type User struct {
	ID uint `gorm:"primarykey"`
	UI string `gorm:"type:varchar(20);uniqueIndex"`
	U  string `gorm:"type:varchar(20);unique"`
}

CREATE TABLE `users` (
	`id` BIGINT UNSIGNED AUTO_INCREMENT,
-	`ui` VARCHAR ( 20 ) UNIQUE,
+	`ui` VARCHAR ( 20 ),
	`u` VARCHAR ( 20 ) UNIQUE,
	PRIMARY KEY ( `id` ),
INDEX `idx_users_deleted_at` ( `deleted_at` ),
UNIQUE INDEX `idx_users_ui` ( `ui` ))

[black-06]

There is no doubt that we need to remove the redundant unique statement.
My question is, should we declare this field as UNIQUE INDEX or UNIQUE CONSTRAINT ?

+ UNIQUE INDEX `idx_users_ui` ( `ui` ))
or
+ CONSTRAINT `idx_users_ui` UNIQUE (`ui`)

[a631807682]

There is no doubt that we need to remove the redundant unique statement. My question is, should we declare this field as UNIQUE INDEX or UNIQUE CONSTRAINT ?

+ UNIQUE INDEX `idx_users_ui` ( `ui` ))
or
+ CONSTRAINT `idx_users_ui` UNIQUE (`ui`)

 UNIQUE INDEX `idx_users_ui` ( `ui` ))

[black-06]

 UNIQUE INDEX `idx_users_ui` ( `ui` ))

I think it should be a constraint.
If it's an index, then we distinguish them without meaning.
And I queried some docs, they said the Unique is constraint

[a631807682]

 UNIQUE INDEX `idx_users_ui` ( `ui` ))

I think it should be a constraint. If it's an index, then we distinguish them without meaning. And I queried some docs, they said the Unique is constraint

When the user uses the uniqueIndex tag, I think what the user wants to create is an index (even if they are the same in most cases), including when we define the uniqueIndex tag, we parse and create a partial index, which may fail when creating a unique constraint?

It's a constraint when the user uses the unique tag, but since we're currently only receiving boolean values, it doesn't make much sense to change it.

https://stackoverflow.com/questions/23542794/postgres-unique-constraint-vs-index
https://dba.stackexchange.com/questions/144/when-should-i-use-a-unique-constraint-instead-of-a-unique-index/147#147?newreg=e5609e35473c4d0b912f18996c349674

[black-06]

When the user uses the uniqueIndex tag, I think what the user wants to create is an index (even if they are the same in most cases), including when we define the uniqueIndex tag, we parse and create a partial index, which may fail when creating a unique constraint?

For uniqueIndex tag, there is no doubt that we created UniqueIndex.

It's a constraint when the user uses the unique tag

+1

but since we're currently only receiving boolean values, it doesn't make much sense to change it.

So we use the unique tag as the unique index?

GORM tag: unique
- unique constraint
+ unique index

But as you shared said, they have some subtle differences, such as error code, foreign key.

summary:

GORM tag	DB
unique	unique constraint (or index?) Here I think it should be a constraint
uniqueIndex	unique index
index	index
index:,unique	unique index

[a631807682]

So we use the unique tag as the unique index?

It should be unique constraint , but since we don't support multiple columns and we don't support patch constraint , so I don't think it makes sense to change it, can we change it in a later version or PR? Or changing it now will make subsequent work becomes easier?

#5559 (comment)

[a631807682]

black-06 proposes that if unique is defined as unique index, it will be indistinguishable (and compared) in database query, and the database query result unique and uniqueIndex tags are the same (can't resolve #5401), so unique needs to be changed into a constraint.

Record this comment.

[starudream]

Are there any plans for an upcoming release?

[black-06]

I found that ci of GORM and drivers are deadlocked, they will never succeed.

So I will split this mr.

[black-06]

I rolled back a small part of them, otherwise this mr would never have passed the ci.
After that, I will modify Dialectors' mr,
when they are merged, I will reopen a new mr to apply the part that was rolled back.

Guys, now that ci is successful, let's review and merge it

[a631807682]

I rolled back a small part of them, otherwise this mr would never have passed the ci. After that, I will modify Dialectors' mr, when they are merged, I will reopen a new mr to apply the part that was rolled back.

Guys, now that ci is successful, let's review and merge it

Can you rebase 9ad102b - ab75e42 to one commit? It has been too long since I approved these changes. I just want to see what has been changed after that.

[black-06]

@a631807682 I have compressed them, please take time to review~