GORM Playground Link
https://github.com/go-gorm/playground/pull/32
Description
Scanning join results into a struct containing nested gorm models fails where nested structs share field names.
In v1, Scanning would unmarshal correctly into each struct provided the order in the composite struct matched the order of the selects. This behaviour is broken in v2.
Creating relationships by nesting stucts when declaring table models structs may not always possible.
Comment From: pkpfr
Has there been any progress on this? The root cause appears to be when parsing the schema, the field names are held in a map[string]interface{} (Schema.FieldsByName and Schema.FieldsByDBName).
This will be problematic for any cases where structs with identical field names are present in the destination object.
A workaround is to manually scan the rows, but this is verbose and difficult to maintain.
Comment From: pkpfr
Since the order in Schema.Fields[] should match that of the selects, then maybe you could iterate over the fields instead of using maps.
fieldsIndex := 0
for idx, column := range columns {
if idx == len(Schema.Fields) || fieldsIndex == len(Schema.Fields) {
values[idx] = &sql.RawBytes{}
continue
}
for _, field := range Schema.Fields[fieldsIndex:] {
if (field.DBName == column || field.Name == column) && field.Readable {
values[idx] = reflect.New(reflect.PtrTo(field.IndirectFieldType)).Interface()
fieldsIndex++
break
}
if names := strings.Split(column, "__"); len(names) > 1 {
if rel, ok := Schema.Relationships.Relations[names[0]]; ok {
if field := rel.FieldSchema.LookUpField(strings.Join(names[1:], "__")); field != nil && field.Readable {
values[idx] = reflect.New(reflect.PtrTo(field.IndirectFieldType)).Interface()
fieldsIndex++
break
}
}
}
}
if values[idx] == nil {
values[idx] = &sql.RawBytes{}
}
}
db.RowsAffected++
db.AddError(rows.Scan(values...))
fieldsIndex = 0
for idx, column := range columns {
if idx == len(Schema.Fields) || fieldsIndex == len(Schema.Fields) {
continue
}
for _, field := range Schema.Fields[fieldsIndex:] {
if (field.DBName == column || field.Name == column) && field.Readable {
field.Set(db.Statement.ReflectValue, values[idx])
fieldsIndex++
break
}
if names := strings.Split(column, "__"); len(names) > 1 {
if rel, ok := Schema.Relationships.Relations[names[0]]; ok {
relValue := rel.Field.ReflectValueOf(db.Statement.ReflectValue)
if field := rel.FieldSchema.LookUpField(strings.Join(names[1:], "__")); field != nil && field.Readable {
value := reflect.ValueOf(values[idx]).Elem()
if relValue.Kind() == reflect.Ptr && relValue.IsNil() {
if value.IsNil() {
fieldsIndex++
break
}
relValue.Set(reflect.New(relValue.Type().Elem()))
}
field.Set(relValue, values[idx])
fieldsIndex++
break
}
}
}
}
}
Comment From: jinzhu
The above code won't work for normal cases as the columns may have different orders than Fields, and it would be even trickier as GORM V2 allows users override embedded struct's field.
So I am still thinking is it worthy to support this fragile, rarely used feature by slowing down all other queries... or any better solution...
Comment From: pkpfr
I have submitted a pull request with more details.
A couple of other options could be: - an additional Scan method, E.g. ScanFields or ScanSlice - custom tags for the structs, that override the alias - this would allow scanning into nested structs for native methods such as First, etc.
I disagree that it is a "rarely used feature". Any developer having to consume someone else's database or use third party models would benefit from this. Overlapping field names are extremely common and the majority of dialects do not automatically prefix column names. So the use of a map, without consideration of the table name doesn't feel right.
Comment From: jinzhu
Hello @pkpfr
Have you tried Preload with Join? this seems like a better solution, clear & not fragile
http://v2.gorm.io/docs/preload.html#Joins-Preloading
Comment From: pkpfr
How would that work where no relationships are defined in the structs?
On 28 Jul 2020, at 10:35 AM, Jinzhu notifications@github.com wrote:
Hello @pkpfr https://github.com/pkpfr Have you tried Preload with Join? this seems like a better solution, clear & not fragile
http://v2.gorm.io/docs/preload.html#Joins-Preloading http://v2.gorm.io/docs/preload.html#Joins-Preloading — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/go-gorm/gorm/issues/3161#issuecomment-664740821, or unsubscribe https://github.com/notifications/unsubscribe-auth/AM6CA3FQ64C4PLJCVWQFGK3R5Y2PRANCNFSM4O6MPTRQ.
Comment From: jinzhu
How would that work where no relationships are defined in the structs? … On 28 Jul 2020, at 10:35 AM, Jinzhu @.***> wrote: Hello @pkpfr https://github.com/pkpfr Have you tried Preload with Join? this seems like a better solution, clear & not fragile http://v2.gorm.io/docs/preload.html#Joins-Preloading http://v2.gorm.io/docs/preload.html#Joins-Preloading — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3161 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AM6CA3FQ64C4PLJCVWQFGK3R5Y2PRANCNFSM4O6MPTRQ.
If you can use joins, there is a relationship?
Comment From: pkpfr
In the documentation it is unclear what “Company”, “Manager” and “Account” refer to. How have they been defined? How are join fields specified if not joining on ID? What SQL would the examples output?
In the examples, passing in a user object and and user keys. What is the purpose of the joins since First or Find would do exactly the same without the Joins.
Alternatively, could you show how I would do this in my playground example?
On 31 Jul 2020, at 13:21, Jinzhu notifications@github.com wrote:
How would that work where no relationships are defined in the structs? … On 28 Jul 2020, at 10:35 AM, Jinzhu @.***> wrote: Hello @pkpfr https://github.com/pkpfr Have you tried Preload with Join? this seems like a better solution, clear & not fragile http://v2.gorm.io/docs/preload.html#Joins-Preloading http://v2.gorm.io/docs/preload.html#Joins-Preloading — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3161 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AM6CA3FQ64C4PLJCVWQFGK3R5Y2PRANCNFSM4O6MPTRQ.
If you can use joins, there is a relationship?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.
Comment From: jinzhu
Hi @pkpfr
Check out the following code, don't forgot to update to the latest code which doesn't require you to write the TableName method for the Composite struct.
Really appreciated for your report
type Thing1 struct {
gorm.Model
Name string `gorm:"size:20"`
One int
}
type Thing2 struct {
gorm.Model
Name string `gorm:"size:20"`
Two int
}
type Thing3 struct {
gorm.Model
Name string `gorm:"size:20"`
Three int
}
type Composite struct {
Thing1
Thing2 Thing2 `gorm:"foreignKey:ID;references:ID"`
Thing3 Thing3 `gorm:"foreignKey:ID;references:ID"`
}
func TestGORM(t *testing.T) {
result := &Composite{}
if err := DB.Migrator().DropTable(&Thing1{}, &Thing2{}, &Thing3{}); err != nil {
t.Errorf("Failed, got error: %v", err)
}
if err := DB.AutoMigrate(&Thing1{}, &Thing2{}, &Thing3{}); err != nil {
t.Errorf("Failed, got error: %v", err)
}
DB.Create(&Thing1{
Name: "Thing 1",
One: 1,
})
DB.Create(&Thing2{
Name: "Thing 2",
Two: 2,
})
DB.Create(&Thing3{
Name: "Thing 3",
Three: 3,
})
DB.Table("thing1").Joins("Thing2").Joins("Thing3").Find(result)
}
Comment From: pkpfr
Thanks. This works for simple queries and relationships, but in production, the relationships are much more complex.
Take this query as an example:
q := "SELECT " +
"products.*, product_subscriptions.*, billing_profiles.* " +
"FROM product_subscriptions " +
"INNER JOIN products ON products.event_type = product_subscriptions.event_type AND products.product_variant = product_subscriptions.product_variant " +
"INNER JOIN billing_profiles ON product_subscriptions.bid = billing_profiles.id " +
"INNER JOIN companies ON billing_profiles.cid = companies.id " +
"WHERE " +
"companies.status & ? > 0 AND " + // company is a production account
"companies.status & ? > 0 AND " + // company is active
"companies.status & ? = 0 AND " + // company is not suspended
"billing_profiles.current_billing_period_end > ? " // billing profiles that aren't rolling over until after 4 hours from now
In cases like this, it doesn't make sense to use Gorm to construct the query, and I'm not sure it would be possible to specify the relationships in struct tags.
This issue only applies to raw queries. So maybe modifying the raw scanner to read the table from the column names?
Comment From: ganeshvaltix
This is a valid issue. With V1 gorm, we used Scan/Find to read columns from multiple tables into a structure with gorm ignored fields, which worked fine. With V2 gorm, the same doesnt work as the gorm ignored fields are not considered readable.
Comment From: pkpfr
We dropped Gorm, because of this issue. It's fine for simple projects and beginners, but it can't cope with sufficient complexity to be suitable for enterprise applications.
Comment From: jinzhu
This is a valid issue. With V1 gorm, we used Scan/Find to read columns from multiple tables into a structure with gorm ignored fields, which worked fine. With V2 gorm, the same doesnt work as the gorm ignored fields are not considered readable.
Check out field permission support in V2 https://gorm.io/docs/models.html#Field-Level-Permission , you can mark the field as readonly.
We dropped Gorm, because of this issue. It's fine for simple projects and beginners, but it can't cope with sufficient complexity to be suitable for enterprise applications.
There is a raw SQL builder, GORM can support everything that database/sql support. https://gorm.io/docs/sql_builder.html
And products.*, product_subscriptions.*, billing_profiles.* is not really suit for enterprise projects, you should write SELECT like products.id as product_id, products.name as product_name to make things direct & clear & maintainable
Comment From: pkpfr
And products., product_subscriptions., billing_profiles.* is not really suit for enterprise projects, you should write SELECT like products.id as product_id, products.name as product_name to make things direct & clear & maintainable
This isn't the issue, you could rewrite the query with defined column names and you still have the same problem. The challenge is for a query like above, you cannot marshal the result into anything. Your scanner uses a map for the field names, which means that columns in different tables that share the same column name get overwritten in the map (as proven in the test case above). Sure, Gorm can construct the query, but if we have to manually process the result every time, then we may as well use the native SQL libraries.
Gorm cannot marshal output for multiple tables into a composite struct unless all relationships are defined, which is not always possible or practical. The v1 scanner could, but the v2 scanner cannot.
Comment From: jinzhu
This isn't the issue, you could rewrite the query with defined column names and you still have the same problem.
If you use defined column names, it would be a unique one, then no issue anymore.
The v1 scanner could, but the v2 scanner cannot.
V1 did some tricky thing, if there are some columns using the same name, it will skip the first found field, and lookup the next field using the name. which make the scan results hard to predict for normal developers, so I would like to abandon its support, here is an example:
type Thing1 struct {
ID uint
Name string
}
type Thing2 Thing1
type Thing3 Thing1
type Composite struct {
Thing1
Thing2
Thing3
}
// works in V1
SELECT thing1.*, thing2.*, thing3.* FROM xxx;
// won't works, will scan thing2 data to Thing1, thing1 data to Thing2, which can't be fixed based on the information we can get
SELECT thing2.*, thing1.*, thing3.* FROM xxx;
// won't works, will scan thing2's name to Thing1, also can't be fixed
SELECT thing1.id, thing2.*, thing3.* FROM XXX
// and it possible to cause many similar bug, which I think we should avoid developer to do that before stepping into the hole
then we may as well use the native SQL libraries.
GORM is not trying to resolve 100% cases, but would like to save your time in 99% of them, and allow you to support 100% cases.
Comment From: ganeshvaltix
Check out field permission support in V2 https://gorm.io/docs/models.html#Field-Level-Permission , you can mark the field as readonly.
We tried that. Issue is that Automigrate adds those fields to the db.
Comment From: jinzhu
With V1 gorm, we used Scan/Find to read columns from multiple tables into a structure with gorm ignored fields, which worked fine
You are not expected to use one struct to migrate those tables?
Comment From: ganeshvaltix
With V1 gorm, we used Scan/Find to read columns from multiple tables into a structure with gorm ignored fields, which worked fine
You are not expected to use one struct to migrate those tables?
We are using a single struct. If I have a field defined as readonly with the attribute gorm:"->". gorm AutoMigrate actually adds the columns to db. My intent is to use that column as a dynamic column for reading data into it temporarily (and not present in the db)
Comment From: VishwasShashidhar
@jinzhu This is almost a year old, but, I agree with @pkpfr that it is not an uncommon use case. I ran into the same issue joining 5 tables, and got stuck for 2 days going through the gorm code to understand what is happening. Finally found this issue.
Either this use case should be supported or the limitation should be documented for others to save time.
Comment From: gm9510
This will be problematic for any cases where structs with identical field names are present in the destination object.
Same problem, Several Joins with tables that have the same names, the only way is to map the fields manually, Sad that there is no alternative :(.
Comment From: amavis442
Use an example like doctrine (i know that it is php). Internally you get prefixes for tables so you can use tables that have columns with the same name (which is not uncommon, like every tabel has an id col)