Skip to content

Commit 315f65b

Browse files
rahul2393rahul2393
authored
fix(spanner): decoding spanner rows using SelectAll should map values in correct annotations (#13301)
Fix: #13299
1 parent d1224e8 commit 315f65b

File tree

2 files changed

+179
-39
lines changed

2 files changed

+179
-39
lines changed

‎spanner/row.go‎

Lines changed: 69 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,9 @@ func SelectAll(rows rowIterator, destination interface{}, options ...DecodeOptio
460460

461461
isPrimitive := itemType.Kind() != reflect.Struct
462462
var pointers []interface{}
463+
var pointerIndexes [][]int
464+
var pointerLookup map[string]int
465+
var orderedPointers []interface{}
463466
isFirstRow := true
464467
var err error
465468
return rows.Do(func(row *Row) error {
@@ -469,9 +472,10 @@ func SelectAll(rows rowIterator, destination interface{}, options ...DecodeOptio
469472
defer func() {
470473
isFirstRow = false
471474
}()
472-
if pointers, err = structPointers(sliceItem.Elem(), row.fields, s.Lenient); err != nil {
475+
if pointers, pointerIndexes, pointerLookup, err = structPointers(sliceItem.Elem(), row.fields, s.Lenient); err != nil {
473476
return err
474477
}
478+
orderedPointers = make([]interface{}, len(pointers))
475479
}
476480
defer func() {
477481
for _, ptr := range pointers {
@@ -482,28 +486,50 @@ func SelectAll(rows rowIterator, destination interface{}, options ...DecodeOptio
482486
}
483487
}
484488
}()
489+
if len(orderedPointers) != len(row.fields) {
490+
orderedPointers = make([]interface{}, len(row.fields))
491+
}
492+
for i, col := range row.fields {
493+
idx, ok := pointerLookup[strings.ToLower(col.GetName())]
494+
if !ok {
495+
if !s.Lenient {
496+
return errNoOrDupGoField(sliceItem.Elem(), col.GetName())
497+
}
498+
orderedPointers[i] = nil
499+
continue
500+
}
501+
orderedPointers[i] = pointers[idx]
502+
}
485503
} else if isPrimitive {
486504
if len(row.fields) > 1 && !s.Lenient {
487505
return errTooManyColumns()
488506
}
489507
pointers = []interface{}{sliceItem.Interface()}
490508
}
491-
if len(pointers) == 0 {
509+
var rowPointers []interface{}
510+
if !isPrimitive {
511+
rowPointers = orderedPointers
512+
} else {
513+
rowPointers = pointers
514+
}
515+
if len(rowPointers) == 0 {
492516
return nil
493517
}
494-
err = row.Columns(pointers...)
518+
err = row.Columns(rowPointers...)
495519
if err != nil {
496520
return err
497521
}
498522
if !isPrimitive {
499523
e := sliceItem.Elem()
500-
idx := 0
501-
for _, p := range pointers {
524+
for i, p := range pointers {
502525
if p == nil {
503526
continue
504527
}
505-
e.Field(idx).Set(reflect.ValueOf(p).Elem())
506-
idx++
528+
indexPath := pointerIndexes[i]
529+
if len(indexPath) == 0 {
530+
continue
531+
}
532+
e.FieldByIndex(indexPath).Set(reflect.ValueOf(p).Elem())
507533
}
508534
}
509535
var elemVal reflect.Value
@@ -524,40 +550,53 @@ func SelectAll(rows rowIterator, destination interface{}, options ...DecodeOptio
524550
})
525551
}
526552

527-
func structPointers(sliceItem reflect.Value, cols []*sppb.StructType_Field, lenient bool) ([]interface{}, error) {
553+
type fieldInfo struct {
554+
value reflect.Value
555+
indexPath []int
556+
}
557+
558+
func structPointers(sliceItem reflect.Value, cols []*sppb.StructType_Field, lenient bool) ([]interface{}, [][]int, map[string]int, error) {
528559
pointers := make([]interface{}, 0, len(cols))
529-
fieldTag := make(map[string]reflect.Value, len(cols))
530-
initFieldTag(sliceItem, &fieldTag)
560+
indexes := make([][]int, 0, len(cols))
561+
fieldTag := make(map[string]fieldInfo, len(cols))
562+
initFieldTag(sliceItem, &fieldTag, nil)
563+
lookup := make(map[string]int, len(cols))
531564

532565
for i, colName := range cols {
533566
if colName.Name == "" {
534-
return nil, errColNotFound(fmt.Sprintf("column %d", i))
567+
return nil, nil, nil, errColNotFound(fmt.Sprintf("column %d", i))
535568
}
536569

537-
var fieldVal reflect.Value
538-
if v, ok := fieldTag[strings.ToLower(colName.GetName())]; ok {
539-
fieldVal = v
540-
} else {
570+
var info fieldInfo
571+
var ok bool
572+
if info, ok = fieldTag[strings.ToLower(colName.GetName())]; !ok {
541573
if !lenient {
542-
return nil, errNoOrDupGoField(sliceItem, colName.GetName())
574+
return nil, nil, nil, errNoOrDupGoField(sliceItem, colName.GetName())
575+
}
576+
if structField, okField := sliceItem.Type().FieldByName(colName.GetName()); okField {
577+
fieldVal := sliceItem.FieldByIndex(structField.Index)
578+
if fieldVal.CanSet() {
579+
info = fieldInfo{value: fieldVal, indexPath: append([]int(nil), structField.Index...)}
580+
}
543581
}
544-
fieldVal = sliceItem.FieldByName(colName.GetName())
545582
}
546-
if !fieldVal.IsValid() || !fieldVal.CanSet() {
547-
// have to add if we found a column because Columns() requires
548-
// len(cols) arguments or it will error. This way we can scan to
549-
// a useless pointer
583+
584+
if !info.value.IsValid() || !info.value.CanSet() {
550585
pointers = append(pointers, nil)
586+
indexes = append(indexes, nil)
587+
lookup[strings.ToLower(colName.GetName())] = len(pointers) - 1
551588
continue
552589
}
553590

554-
pointers = append(pointers, fieldVal.Addr().Interface())
591+
pointers = append(pointers, info.value.Addr().Interface())
592+
indexes = append(indexes, info.indexPath)
593+
lookup[strings.ToLower(colName.GetName())] = len(pointers) - 1
555594
}
556-
return pointers, nil
595+
return pointers, indexes, lookup, nil
557596
}
558597

559598
// Initialization the tags from struct.
560-
func initFieldTag(sliceItem reflect.Value, fieldTagMap *map[string]reflect.Value) {
599+
func initFieldTag(sliceItem reflect.Value, fieldTagMap *map[string]fieldInfo, path []int) {
561600
typ := sliceItem.Type()
562601

563602
for i := 0; i < sliceItem.NumField(); i++ {
@@ -569,11 +608,12 @@ func initFieldTag(sliceItem reflect.Value, fieldTagMap *map[string]reflect.Value
569608
if !exported && !fieldType.Anonymous {
570609
continue
571610
}
611+
currentPath := append(path, i)
572612
if fieldType.Type.Kind() == reflect.Struct {
573613
// found an embedded struct
574614
if fieldType.Anonymous {
575615
sliceItemOfAnonymous := sliceItem.Field(i)
576-
initFieldTag(sliceItemOfAnonymous, fieldTagMap)
616+
initFieldTag(sliceItemOfAnonymous, fieldTagMap, currentPath)
577617
continue
578618
}
579619
}
@@ -584,6 +624,9 @@ func initFieldTag(sliceItem reflect.Value, fieldTagMap *map[string]reflect.Value
584624
if name == "" {
585625
name = fieldType.Name
586626
}
587-
(*fieldTagMap)[strings.ToLower(name)] = sliceItem.Field(i)
627+
(*fieldTagMap)[strings.ToLower(name)] = fieldInfo{
628+
value: sliceItem.Field(i),
629+
indexPath: append([]int(nil), currentPath...),
630+
}
588631
}
589632
}

‎spanner/row_test.go‎

Lines changed: 110 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2289,10 +2289,16 @@ func TestSelectAll(t *testing.T) {
22892289
Col4 time.Time `spanner:"TAG4"`
22902290
}
22912291

2292+
type testStructWithSimpleTag struct {
2293+
Col1 int64 `spanner:"tag1"`
2294+
Col2 int64 `spanner:"tag2"`
2295+
Col3 int64 `spanner:"tag3"`
2296+
Col4 int64 `spanner:"tag4"`
2297+
}
22922298
type Address struct {
2293-
Street string
2294-
ZipCode string
2295-
City string
2299+
AddressZip string `spanner:"ZipCode"`
2300+
AddressStreet string `spanner:"Street"`
2301+
AddressCity string `spanner:"City"`
22962302
}
22972303

22982304
type Person struct {
@@ -2542,6 +2548,78 @@ func TestSelectAll(t *testing.T) {
25422548
{Col1: 2, Col2: 2.2, Col3: "value2", Col4: tm.Add(24 * time.Hour)},
25432549
},
25442550
},
2551+
{
2552+
name: "success: using slice of structs with simple spanner tag annotations in different order",
2553+
args: args{
2554+
destination: &[]testStructWithSimpleTag{},
2555+
mock: newMockIterator(
2556+
&Row{
2557+
[]*sppb.StructType_Field{
2558+
{Name: "Tag2", Type: intType()},
2559+
{Name: "Tag1", Type: intType()},
2560+
{Name: "Tag4", Type: intType()},
2561+
{Name: "Tag3", Type: intType()},
2562+
},
2563+
[]*proto3.Value{intProto(2), intProto(1), intProto(4), intProto(3)},
2564+
},
2565+
&Row{
2566+
[]*sppb.StructType_Field{
2567+
{Name: "Tag1", Type: intType()},
2568+
{Name: "Tag2", Type: intType()},
2569+
{Name: "Tag3", Type: intType()},
2570+
{Name: "Tag4", Type: intType()},
2571+
},
2572+
[]*proto3.Value{intProto(1), intProto(2), intProto(3), intProto(4)},
2573+
},
2574+
&Row{
2575+
[]*sppb.StructType_Field{
2576+
{Name: "Tag2", Type: intType()},
2577+
{Name: "Tag1", Type: intType()},
2578+
{Name: "Tag4", Type: intType()},
2579+
{Name: "Tag3", Type: intType()},
2580+
},
2581+
[]*proto3.Value{intProto(2), intProto(1), intProto(4), intProto(3)},
2582+
},
2583+
iterator.Done,
2584+
),
2585+
},
2586+
want: &[]testStructWithSimpleTag{
2587+
{Col1: 1, Col2: 2, Col3: 3, Col4: 4},
2588+
{Col1: 1, Col2: 2, Col3: 3, Col4: 4},
2589+
{Col1: 1, Col2: 2, Col3: 3, Col4: 4},
2590+
},
2591+
},
2592+
{
2593+
name: "success: using slice of structs with spanner tag annotations in different order",
2594+
args: args{
2595+
destination: &[]testStructWithTag{},
2596+
mock: newMockIterator(
2597+
&Row{
2598+
[]*sppb.StructType_Field{
2599+
{Name: "Tag2", Type: floatType()},
2600+
{Name: "Tag1", Type: intType()},
2601+
{Name: "Tag4", Type: timeType()},
2602+
{Name: "Tag3", Type: stringType()},
2603+
},
2604+
[]*proto3.Value{floatProto(1.1), intProto(1), timeProto(tm), stringProto("value")},
2605+
},
2606+
&Row{
2607+
[]*sppb.StructType_Field{
2608+
{Name: "Tag2", Type: floatType()},
2609+
{Name: "Tag1", Type: intType()},
2610+
{Name: "Tag4", Type: timeType()},
2611+
{Name: "Tag3", Type: stringType()},
2612+
},
2613+
[]*proto3.Value{floatProto(2.2), intProto(2), timeProto(tm.Add(24 * time.Hour)), stringProto("value2")},
2614+
},
2615+
iterator.Done,
2616+
),
2617+
},
2618+
want: &[]testStructWithTag{
2619+
{Col1: 1, Col2: 1.1, Col3: "value", Col4: tm},
2620+
{Col1: 2, Col2: 2.2, Col3: "value2", Col4: tm.Add(24 * time.Hour)},
2621+
},
2622+
},
25452623
{
25462624
name: "failure: in case of error destination will have the partial result",
25472625
args: args{
@@ -2675,46 +2753,65 @@ func TestSelectAll(t *testing.T) {
26752753
wantErr: true,
26762754
},
26772755
{
2678-
name: "failure: nested unnamed structs",
2756+
name: "success: nested unnamed structs",
26792757
args: args{
26802758
destination: &[]*PersonEmbedded{},
26812759
mock: newMockIterator(
26822760
&Row{
26832761
[]*sppb.StructType_Field{
26842762
{Name: "Name", Type: stringType()},
2685-
{Name: "Street", Type: stringType()},
2686-
{Name: "ZipCode", Type: stringType()},
26872763
{Name: "City", Type: stringType()},
2764+
{Name: "Street", Type: stringType()},
26882765
{Name: "BirthDate", Type: dateType()},
2766+
{Name: "ZipCode", Type: stringType()},
26892767
},
26902768
[]*proto3.Value{
26912769
stringProto("Name1"),
2692-
stringProto("Street1"),
2693-
stringProto("ZipCode1"),
26942770
stringProto("City1"),
2771+
stringProto("Street1"),
26952772
dateProto(civil.Date{Year: 2000, Month: 11, Day: 14}),
2773+
stringProto("ZipCode1"),
26962774
},
26972775
},
26982776
&Row{
26992777
[]*sppb.StructType_Field{
27002778
{Name: "Name", Type: stringType()},
2701-
{Name: "Street", Type: stringType()},
2702-
{Name: "ZipCode", Type: stringType()},
27032779
{Name: "City", Type: stringType()},
2780+
{Name: "Street", Type: stringType()},
27042781
{Name: "BirthDate", Type: dateType()},
2782+
{Name: "ZipCode", Type: stringType()},
27052783
},
27062784
[]*proto3.Value{
27072785
stringProto("Name2"),
2708-
stringProto("Street2"),
2709-
stringProto("ZipCode2"),
27102786
stringProto("City2"),
2787+
stringProto("Street2"),
27112788
dateProto(civil.Date{Year: 2001, Month: 11, Day: 14}),
2789+
stringProto("ZipCode2"),
27122790
},
27132791
},
27142792
iterator.Done,
27152793
),
27162794
},
2717-
wantPanic: true,
2795+
want: &[]*PersonEmbedded{
2796+
{
2797+
Name: "Name1",
2798+
Address: Address{
2799+
AddressZip: "ZipCode1",
2800+
AddressStreet: "Street1",
2801+
AddressCity: "City1",
2802+
},
2803+
BirthDate: civil.Date{Year: 2000, Month: 11, Day: 14},
2804+
},
2805+
{
2806+
Name: "Name2",
2807+
Address: Address{
2808+
AddressZip: "ZipCode2",
2809+
AddressStreet: "Street2",
2810+
AddressCity: "City2",
2811+
},
2812+
BirthDate: civil.Date{Year: 2001, Month: 11, Day: 14},
2813+
},
2814+
},
27182815
},
27192816
}
27202817
for _, tt := range tests {

0 commit comments

Comments
 (0)