fix: update panic if model is not ptr#6037
Conversation
black-06
force-pushed
the
fix_callback
branch
from
4fa7de2 to
72decd0
Compare
| } | ||
| case reflect.Struct: | ||
| if !db.Statement.ReflectValue.CanAddr() { | ||
| db.Statement.ReflectValue = reflect.New(db.Statement.ReflectValue.Type()).Elem() |
There was a problem hiding this comment.
I think maybe better to raise an error?
There was a problem hiding this comment.
Do you mean that param of Model() method must be ptr?
| fc(reflect.Indirect(db.Statement.ReflectValue.Index(i)).Addr().Interface(), tx) | ||
| if value := reflect.Indirect(db.Statement.ReflectValue.Index(i)); value.CanAddr() { | ||
| fc(value.Addr().Interface(), tx) | ||
| } |
There was a problem hiding this comment.
Should we raise an error if the value is not addressable? it should be ok to return an error in those cases would panic before.
There was a problem hiding this comment.
err = DB.Model(Product5{}).Where("id", p.ID).Update("name", "update_name_2").Error
If so, this usage will raise an error, I'm not sure if it's good, it's up to you.
There was a problem hiding this comment.
err = DB.Model(Product5{}).Where("id", p.ID).Update("name", "update_name_2").ErrorIf so, this usage will raise an error, I'm not sure if it's good, it's up to you.
I thought I sent this message yesterday, but in fact it was pending.... 😨
3 participants
What did this pull request do?
fix #5989.
The same pull request is #5990.
The different is that hook will still be executed when the model is not ptr.
User Case Description
DB.Model(Product5{}).Where("id", p.ID).Update("name", "update_name")