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

fix: AUTOINCREMENT flag cannot apply with PRIMARY KEY #167

Merged
merged 2 commits into from
Oct 8, 2023

Conversation

samuelncui
Copy link
Contributor

@samuelncui samuelncui commented Oct 4, 2023

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

What did this pull request do?

Fix #4760. Currently, sqlite dialector can't apply AUTOINCREMENT to the field type. The reason this method cannot work is if a field is marked as AutoIncrement and PrimaryKey at the same time, when we apply AUTOINCREMENT to data type, it will create SQL like the following:

CREATE TABLE `example_table` (`id` integer PRIMARY KEY AUTOINCREMENT, ..., PRIMARY KEY (`id`))'

This can cause SQL logic error: table "example_table" has more than one primary key (1) error, because there is two PRIMARY KEY definition in this statement. The root cause is hasPrimaryKeyInDataType doesn't check the result of Dialector.DataTypeOf method.

By checking the result of Dialector.DataTypeOf method, a dialector can define its behavior for AUTOINCREMENT & PRIMARY KEY situation.

User Case Description

The following struct:

type ExampleTable struct {
	ID     int64  `gorm:"primaryKey;autoIncrement" json:"id,omitempty"`
	...
}

Used to generate table DDL as:

CREATE TABLE `example_table` (`id` integer, ..., PRIMARY KEY (`id`))'

Which doesn't mark id field as AUTOINCREMENT. Currently, the id field roughly acts like AUTOINCREMENT, but it doesn't in many areas. This implementation may cause some issues like id reusing when deleting rows.

Now, it will generate table DDL as (if sqlite dialector is also changed):

CREATE TABLE `example_table` (`id` integer PRIMARY KEY AUTOINCREMENT, ...)'

@jinzhu jinzhu merged commit 139bd30 into go-gorm:master Oct 8, 2023
4 checks passed
@xuxiaowei-com-cn
Copy link

#168

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.

3 participants