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: prevent interface type array from causing runtime errors #7361

Closed
wants to merge 10 commits into from

Conversation

KEHyeon
Copy link

@KEHyeon KEHyeon commented Jan 26, 2025

  • Do only one thing
  • Non breaking API changes
  • [v] Tested

What did this pull request do?

fix: prevent interface type array from causing runtime errors
It has been modified to find out the value when it is an interface array.

User Case Description

type UserInterface interface {
	GetName() string
}

db.Table("users").Create([]UserInterface{UserToUserInterface(&userData)})

In this case, create is allowed.

@KEHyeon KEHyeon changed the title Interface array fix: prevent interface type array from causing runtime errors Jan 26, 2025
Copy link
Contributor

@daheige daheige left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Author

@KEHyeon KEHyeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thankyou

Copy link
Member

@a631807682 a631807682 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some tests?

@KEHyeon
Copy link
Author

KEHyeon commented Feb 10, 2025

Can you add some tests?

Can I just write test code?

@a631807682
Copy link
Member

Can you add some tests?

Can I just write test code?

You can add unit tests to show what this PR fixes. https://github.com/go-gorm/gorm/blob/master/tests/create_test.go

@KEHyeon
Copy link
Author

KEHyeon commented Feb 15, 2025

I guess I was wrong. If I change it one by one like this, the sideEffect seems to be too big. I thought of another way, so I'll upload the pr again.

@KEHyeon KEHyeon closed this Feb 15, 2025
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