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.

Sign up for GitHub

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

Jump to bottom

fix(spanner): SelectAll struct fields match should be case-insensitive #9417

Merged
merged 3 commits into from Feb 13, 2024
+19 −17
Merged

fix(spanner): SelectAll struct fields match should be case-insensitive #9417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5317325
fix(spanner): SelectAll struct fields match should be case-insensitive
rahul2393 Feb 13, 2024
c9869ef
fix comment
rahul2393 Feb 13, 2024
db48709
fix casing for mock response
rahul2393 Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions spanner/row.go
View file
Open in desktop
Expand Up @@ -19,6 +19,7 @@ package spanner
import (
"fmt"
"reflect"
"strings"

sppb "cloud.google.com/go/spanner/apiv1/spannerpb"
proto3 "github.com/golang/protobuf/ptypes/struct"
Expand Down Expand Up @@ -518,7 +519,7 @@ func structPointers(sliceItem reflect.Value, cols []*sppb.StructType_Field, leni
}

var fieldVal reflect.Value
if v, ok := fieldTag[colName.GetName()]; ok {
if v, ok := fieldTag[strings.ToLower(colName.GetName())]; ok {
fieldVal = v
} else {
if !lenient {
Expand Down Expand Up @@ -565,7 +566,7 @@ func initFieldTag(sliceItem reflect.Value, fieldTagMap *map[string]reflect.Value
continue
}
if name == "" {
name = fieldType.Name
name = strings.ToLower(fieldType.Name)
}
(*fieldTagMap)[name] = sliceItem.Field(i)
}
Expand Down
31 changes: 16 additions & 15 deletions spanner/row_test.go
View file
Open in desktop
Expand Up @@ -2003,7 +2003,8 @@ func TestSelectAll(t *testing.T) {
}
type testStruct struct {
Col1 int64
Col2 float64
// declaring second column in upper case here to verify SelectAll does case-insensitive matching
COL2 float64
Col3 string
Col4 time.Time
}
Expand Down Expand Up @@ -2087,8 +2088,8 @@ func TestSelectAll(t *testing.T) {
},
},
want: &[]testStruct{
{Col1: 1, Col2: 1.1, Col3: "value", Col4: tm},
{Col1: 2, Col2: 2.2, Col3: "value2", Col4: tm.Add(24 * time.Hour)},
{Col1: 1, COL2: 1.1, Col3: "value", Col4: tm},
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here also: Should this not be kept as Col2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this should be COL2, because testStruct is user defined struct and we are asserting that though mock response contained col2 it was successfully decoded in testStruct

Copy link
Contributor

Choose a reason for hiding this comment

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

Eh, yeah, I read that wrong. I thought that this was also a mock iterator, but it's clearly not....

{Col1: 2, COL2: 2.2, Col3: "value2", Col4: tm.Add(24 * time.Hour)},
},
},
{
Expand Down Expand Up @@ -2117,8 +2118,8 @@ func TestSelectAll(t *testing.T) {
},
},
want: &[]*testStruct{
{Col1: 1, Col2: 1.1, Col3: "value"},
{Col1: 2, Col2: 2.2, Col3: "value2"},
{Col1: 1, COL2: 1.1, Col3: "value"},
{Col1: 2, COL2: 2.2, Col3: "value2"},
}},
{
name: "success: when spanner row contains more columns than declared in Go struct but called WithLenient",
Expand All @@ -2142,13 +2143,13 @@ func TestSelectAll(t *testing.T) {
options: []DecodeOptions{WithLenient()},
},
want: &[]*testStruct{
{Col1: 1, Col2: 1.1, Col3: "value", Col4: tm},
{Col1: 1, COL2: 1.1, Col3: "value", Col4: tm},
},
},
{
name: "success: using prefilled destination should append to the destination",
args: args{
destination: &[]*testStruct{{Col1: 3, Col2: 3.3, Col3: "value3"}},
destination: &[]*testStruct{{Col1: 3, COL2: 3.3, Col3: "value3"}},
mock: func(mockIterator *mockRowIterator) {
mockIterator.On("Next").Once().Return(&Row{
[]*sppb.StructType_Field{
Expand All @@ -2171,14 +2172,14 @@ func TestSelectAll(t *testing.T) {
},
},
want: &[]*testStruct{
{Col1: 3, Col2: 3.3, Col3: "value3"},
{Col1: 1, Col2: 1.1, Col3: "value"},
{Col1: 2, Col2: 2.2, Col3: "value2"},
{Col1: 3, COL2: 3.3, Col3: "value3"},
{Col1: 1, COL2: 1.1, Col3: "value"},
{Col1: 2, COL2: 2.2, Col3: "value2"},
}},
{
name: "failure: in case of error destination will have the partial result",
args: args{
destination: &[]*testStruct{{Col1: 3, Col2: 3.3, Col3: "value3"}},
destination: &[]*testStruct{{Col1: 3, COL2: 3.3, Col3: "value3"}},
mock: func(mockIterator *mockRowIterator) {
mockIterator.On("Next").Once().Return(&Row{
[]*sppb.StructType_Field{
Expand All @@ -2194,15 +2195,15 @@ func TestSelectAll(t *testing.T) {
},
},
want: &[]*testStruct{
{Col1: 3, Col2: 3.3, Col3: "value3"},
{Col1: 1, Col2: 1.1, Col3: "value"},
{Col1: 3, COL2: 3.3, Col3: "value3"},
{Col1: 1, COL2: 1.1, Col3: "value"},
},
wantErr: true,
},
{
name: "failure: when spanner row contains more columns than declared in Go struct",
args: args{
destination: &[]*testStruct{{Col1: 3, Col2: 3.3, Col3: "value3"}},
destination: &[]*testStruct{{Col1: 3, COL2: 3.3, Col3: "value3"}},
mock: func(mockIterator *mockRowIterator) {
mockIterator.On("Next").Once().Return(&Row{
[]*sppb.StructType_Field{
Expand All @@ -2219,7 +2220,7 @@ func TestSelectAll(t *testing.T) {
},
},
want: &[]*testStruct{
{Col1: 3, Col2: 3.3, Col3: "value3"},
{Col1: 3, COL2: 3.3, Col3: "value3"},
},
wantErr: true,
},
Expand Down