diff --git a/spanner/row.go b/spanner/row.go index b0c23ee5cf89..68203c595acb 100644 --- a/spanner/row.go +++ b/spanner/row.go @@ -460,6 +460,9 @@ func SelectAll(rows rowIterator, destination interface{}, options ...DecodeOptio isPrimitive := itemType.Kind() != reflect.Struct var pointers []interface{} + var pointerIndexes [][]int + var pointerLookup map[string]int + var orderedPointers []interface{} isFirstRow := true var err error return rows.Do(func(row *Row) error { @@ -469,9 +472,10 @@ func SelectAll(rows rowIterator, destination interface{}, options ...DecodeOptio defer func() { isFirstRow = false }() - if pointers, err = structPointers(sliceItem.Elem(), row.fields, s.Lenient); err != nil { + if pointers, pointerIndexes, pointerLookup, err = structPointers(sliceItem.Elem(), row.fields, s.Lenient); err != nil { return err } + orderedPointers = make([]interface{}, len(pointers)) } defer func() { for _, ptr := range pointers { @@ -482,28 +486,50 @@ func SelectAll(rows rowIterator, destination interface{}, options ...DecodeOptio } } }() + if len(orderedPointers) != len(row.fields) { + orderedPointers = make([]interface{}, len(row.fields)) + } + for i, col := range row.fields { + idx, ok := pointerLookup[strings.ToLower(col.GetName())] + if !ok { + if !s.Lenient { + return errNoOrDupGoField(sliceItem.Elem(), col.GetName()) + } + orderedPointers[i] = nil + continue + } + orderedPointers[i] = pointers[idx] + } } else if isPrimitive { if len(row.fields) > 1 && !s.Lenient { return errTooManyColumns() } pointers = []interface{}{sliceItem.Interface()} } - if len(pointers) == 0 { + var rowPointers []interface{} + if !isPrimitive { + rowPointers = orderedPointers + } else { + rowPointers = pointers + } + if len(rowPointers) == 0 { return nil } - err = row.Columns(pointers...) + err = row.Columns(rowPointers...) if err != nil { return err } if !isPrimitive { e := sliceItem.Elem() - idx := 0 - for _, p := range pointers { + for i, p := range pointers { if p == nil { continue } - e.Field(idx).Set(reflect.ValueOf(p).Elem()) - idx++ + indexPath := pointerIndexes[i] + if len(indexPath) == 0 { + continue + } + e.FieldByIndex(indexPath).Set(reflect.ValueOf(p).Elem()) } } var elemVal reflect.Value @@ -524,40 +550,53 @@ func SelectAll(rows rowIterator, destination interface{}, options ...DecodeOptio }) } -func structPointers(sliceItem reflect.Value, cols []*sppb.StructType_Field, lenient bool) ([]interface{}, error) { +type fieldInfo struct { + value reflect.Value + indexPath []int +} + +func structPointers(sliceItem reflect.Value, cols []*sppb.StructType_Field, lenient bool) ([]interface{}, [][]int, map[string]int, error) { pointers := make([]interface{}, 0, len(cols)) - fieldTag := make(map[string]reflect.Value, len(cols)) - initFieldTag(sliceItem, &fieldTag) + indexes := make([][]int, 0, len(cols)) + fieldTag := make(map[string]fieldInfo, len(cols)) + initFieldTag(sliceItem, &fieldTag, nil) + lookup := make(map[string]int, len(cols)) for i, colName := range cols { if colName.Name == "" { - return nil, errColNotFound(fmt.Sprintf("column %d", i)) + return nil, nil, nil, errColNotFound(fmt.Sprintf("column %d", i)) } - var fieldVal reflect.Value - if v, ok := fieldTag[strings.ToLower(colName.GetName())]; ok { - fieldVal = v - } else { + var info fieldInfo + var ok bool + if info, ok = fieldTag[strings.ToLower(colName.GetName())]; !ok { if !lenient { - return nil, errNoOrDupGoField(sliceItem, colName.GetName()) + return nil, nil, nil, errNoOrDupGoField(sliceItem, colName.GetName()) + } + if structField, okField := sliceItem.Type().FieldByName(colName.GetName()); okField { + fieldVal := sliceItem.FieldByIndex(structField.Index) + if fieldVal.CanSet() { + info = fieldInfo{value: fieldVal, indexPath: append([]int(nil), structField.Index...)} + } } - fieldVal = sliceItem.FieldByName(colName.GetName()) } - if !fieldVal.IsValid() || !fieldVal.CanSet() { - // have to add if we found a column because Columns() requires - // len(cols) arguments or it will error. This way we can scan to - // a useless pointer + + if !info.value.IsValid() || !info.value.CanSet() { pointers = append(pointers, nil) + indexes = append(indexes, nil) + lookup[strings.ToLower(colName.GetName())] = len(pointers) - 1 continue } - pointers = append(pointers, fieldVal.Addr().Interface()) + pointers = append(pointers, info.value.Addr().Interface()) + indexes = append(indexes, info.indexPath) + lookup[strings.ToLower(colName.GetName())] = len(pointers) - 1 } - return pointers, nil + return pointers, indexes, lookup, nil } // Initialization the tags from struct. -func initFieldTag(sliceItem reflect.Value, fieldTagMap *map[string]reflect.Value) { +func initFieldTag(sliceItem reflect.Value, fieldTagMap *map[string]fieldInfo, path []int) { typ := sliceItem.Type() for i := 0; i < sliceItem.NumField(); i++ { @@ -569,11 +608,12 @@ func initFieldTag(sliceItem reflect.Value, fieldTagMap *map[string]reflect.Value if !exported && !fieldType.Anonymous { continue } + currentPath := append(path, i) if fieldType.Type.Kind() == reflect.Struct { // found an embedded struct if fieldType.Anonymous { sliceItemOfAnonymous := sliceItem.Field(i) - initFieldTag(sliceItemOfAnonymous, fieldTagMap) + initFieldTag(sliceItemOfAnonymous, fieldTagMap, currentPath) continue } } @@ -584,6 +624,9 @@ func initFieldTag(sliceItem reflect.Value, fieldTagMap *map[string]reflect.Value if name == "" { name = fieldType.Name } - (*fieldTagMap)[strings.ToLower(name)] = sliceItem.Field(i) + (*fieldTagMap)[strings.ToLower(name)] = fieldInfo{ + value: sliceItem.Field(i), + indexPath: append([]int(nil), currentPath...), + } } } diff --git a/spanner/row_test.go b/spanner/row_test.go index 78e2b36c016d..462899515038 100644 --- a/spanner/row_test.go +++ b/spanner/row_test.go @@ -2289,10 +2289,16 @@ func TestSelectAll(t *testing.T) { Col4 time.Time `spanner:"TAG4"` } + type testStructWithSimpleTag struct { + Col1 int64 `spanner:"tag1"` + Col2 int64 `spanner:"tag2"` + Col3 int64 `spanner:"tag3"` + Col4 int64 `spanner:"tag4"` + } type Address struct { - Street string - ZipCode string - City string + AddressZip string `spanner:"ZipCode"` + AddressStreet string `spanner:"Street"` + AddressCity string `spanner:"City"` } type Person struct { @@ -2542,6 +2548,78 @@ func TestSelectAll(t *testing.T) { {Col1: 2, Col2: 2.2, Col3: "value2", Col4: tm.Add(24 * time.Hour)}, }, }, + { + name: "success: using slice of structs with simple spanner tag annotations in different order", + args: args{ + destination: &[]testStructWithSimpleTag{}, + mock: newMockIterator( + &Row{ + []*sppb.StructType_Field{ + {Name: "Tag2", Type: intType()}, + {Name: "Tag1", Type: intType()}, + {Name: "Tag4", Type: intType()}, + {Name: "Tag3", Type: intType()}, + }, + []*proto3.Value{intProto(2), intProto(1), intProto(4), intProto(3)}, + }, + &Row{ + []*sppb.StructType_Field{ + {Name: "Tag1", Type: intType()}, + {Name: "Tag2", Type: intType()}, + {Name: "Tag3", Type: intType()}, + {Name: "Tag4", Type: intType()}, + }, + []*proto3.Value{intProto(1), intProto(2), intProto(3), intProto(4)}, + }, + &Row{ + []*sppb.StructType_Field{ + {Name: "Tag2", Type: intType()}, + {Name: "Tag1", Type: intType()}, + {Name: "Tag4", Type: intType()}, + {Name: "Tag3", Type: intType()}, + }, + []*proto3.Value{intProto(2), intProto(1), intProto(4), intProto(3)}, + }, + iterator.Done, + ), + }, + want: &[]testStructWithSimpleTag{ + {Col1: 1, Col2: 2, Col3: 3, Col4: 4}, + {Col1: 1, Col2: 2, Col3: 3, Col4: 4}, + {Col1: 1, Col2: 2, Col3: 3, Col4: 4}, + }, + }, + { + name: "success: using slice of structs with spanner tag annotations in different order", + args: args{ + destination: &[]testStructWithTag{}, + mock: newMockIterator( + &Row{ + []*sppb.StructType_Field{ + {Name: "Tag2", Type: floatType()}, + {Name: "Tag1", Type: intType()}, + {Name: "Tag4", Type: timeType()}, + {Name: "Tag3", Type: stringType()}, + }, + []*proto3.Value{floatProto(1.1), intProto(1), timeProto(tm), stringProto("value")}, + }, + &Row{ + []*sppb.StructType_Field{ + {Name: "Tag2", Type: floatType()}, + {Name: "Tag1", Type: intType()}, + {Name: "Tag4", Type: timeType()}, + {Name: "Tag3", Type: stringType()}, + }, + []*proto3.Value{floatProto(2.2), intProto(2), timeProto(tm.Add(24 * time.Hour)), stringProto("value2")}, + }, + iterator.Done, + ), + }, + want: &[]testStructWithTag{ + {Col1: 1, Col2: 1.1, Col3: "value", Col4: tm}, + {Col1: 2, Col2: 2.2, Col3: "value2", Col4: tm.Add(24 * time.Hour)}, + }, + }, { name: "failure: in case of error destination will have the partial result", args: args{ @@ -2675,46 +2753,65 @@ func TestSelectAll(t *testing.T) { wantErr: true, }, { - name: "failure: nested unnamed structs", + name: "success: nested unnamed structs", args: args{ destination: &[]*PersonEmbedded{}, mock: newMockIterator( &Row{ []*sppb.StructType_Field{ {Name: "Name", Type: stringType()}, - {Name: "Street", Type: stringType()}, - {Name: "ZipCode", Type: stringType()}, {Name: "City", Type: stringType()}, + {Name: "Street", Type: stringType()}, {Name: "BirthDate", Type: dateType()}, + {Name: "ZipCode", Type: stringType()}, }, []*proto3.Value{ stringProto("Name1"), - stringProto("Street1"), - stringProto("ZipCode1"), stringProto("City1"), + stringProto("Street1"), dateProto(civil.Date{Year: 2000, Month: 11, Day: 14}), + stringProto("ZipCode1"), }, }, &Row{ []*sppb.StructType_Field{ {Name: "Name", Type: stringType()}, - {Name: "Street", Type: stringType()}, - {Name: "ZipCode", Type: stringType()}, {Name: "City", Type: stringType()}, + {Name: "Street", Type: stringType()}, {Name: "BirthDate", Type: dateType()}, + {Name: "ZipCode", Type: stringType()}, }, []*proto3.Value{ stringProto("Name2"), - stringProto("Street2"), - stringProto("ZipCode2"), stringProto("City2"), + stringProto("Street2"), dateProto(civil.Date{Year: 2001, Month: 11, Day: 14}), + stringProto("ZipCode2"), }, }, iterator.Done, ), }, - wantPanic: true, + want: &[]*PersonEmbedded{ + { + Name: "Name1", + Address: Address{ + AddressZip: "ZipCode1", + AddressStreet: "Street1", + AddressCity: "City1", + }, + BirthDate: civil.Date{Year: 2000, Month: 11, Day: 14}, + }, + { + Name: "Name2", + Address: Address{ + AddressZip: "ZipCode2", + AddressStreet: "Street2", + AddressCity: "City2", + }, + BirthDate: civil.Date{Year: 2001, Month: 11, Day: 14}, + }, + }, }, } for _, tt := range tests {