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: return id which have type string after created #5477

Merged
merged 1 commit into from Sep 22, 2022

Conversation

nohattee
Copy link
Contributor

@nohattee nohattee commented Jul 1, 2022

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

What did this pull request do?

Returning the ID which has uuid.UUID or String type after created

User Case Description

I have a sql script like the below and run on Postgres DB:

CREATE TABLE "users" (
    "id" UUID NOT NULL DEFAULT gen_random_uuid(),
    "email" TEXT NOT NULL,
    "password" TEXT,
    "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
    "updated_at" TIMESTAMP(3) NOT NULL,
    "deleted_at" TIMESTAMP(3),
    CONSTRAINT "users_pkey" primary KEY ("id")
);

In the golang code, I created a struct like this:

type User struct {
	ID        uuid.UUID      `json:"id"`
	Email     string         `json:"email"`
	Password  string         `json:"password"`
	CreatedAt time.Time      `json:"created_at"`
	UpdatedAt time.Time      `json:"updated_at"`
	DeletedAt gorm.DeletedAt `json:"deleted_at"`
}

I use this method to create user:

user := &User{
         Email:    newUser.Email,
         Password: newUser.Password,
}
result := db.Select("Email", "Password").Create(user)
fmt.Println(user)

The ID was not return after created, so that the user.ID has zero value (not expected).

@nohattee nohattee force-pushed the master branch 2 times, most recently from 231f90b to efce89b Compare July 2, 2022 02:59
@jinzhu
Copy link
Member

jinzhu commented Jul 4, 2022

Hi @nohattee

Can you add some tests?

@nohattee
Copy link
Contributor Author

nohattee commented Jul 4, 2022

Hi @jinzhu
For adding test cases, I need to create a table that has a DEFAULT gen_random_uuid() value like this:

CREATE TABLE "users" (
    "id" UUID NOT NULL DEFAULT gen_random_uuid(),
    "email" TEXT NOT NULL,
    "password" TEXT,
    "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
    "updated_at" TIMESTAMP(3) NOT NULL,
    "deleted_at" TIMESTAMP(3),
    CONSTRAINT "users_pkey" primary KEY ("id")
);

Do you know how to create a thing like that? Please show me and I will add more test cases

@jinzhu
Copy link
Member

jinzhu commented Jul 5, 2022

Hi , you can define a struct with default tag for example:

https://gorm.io/docs/create.html#Default-Values

Also refer tests/postgres_test.go

@nohattee
Copy link
Contributor Author

nohattee commented Jul 7, 2022

hi @jinzhu , I just added some test cases. Please help me double-check it.

@a631807682
Copy link
Member

a631807682 commented Jul 11, 2022

Hi @nohattee
Can the difference be reflected in the test? Can a test case fail before the change and succeed after the change?
It seems to me that it can be handled here https://github.com/go-gorm/gorm/blob/master/schema/schema.go#L227

@nohattee-manabie
Copy link

Hi @a631807682
This case is kind of hard to reproduce. I need to generate a table with the SQL script and the struct like User Case Description . But it seems there is no way to do that in gorm

@a631807682
Copy link
Member

Hi @a631807682 This case is kind of hard to reproduce. I need to generate a table with the SQL script and the struct like User Case Description . But it seems there is no way to do that in gorm

Do you mean there is a difference between ddl and model? you can use Table(..).AutoMigrate() to create table, and use other model to create records.
refer to https://github.com/go-gorm/gorm/blob/master/tests/migrate_test.go#L627

@jinzhu jinzhu merged commit 328f301 into go-gorm:master Sep 22, 2022
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.

None yet

4 participants