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.

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

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

Merged
merged 3 commits into from Feb 13, 2024

Conversation

rahul2393
Copy link
Contributor

Fixes: #9415

row.ToStruct does case-insensitive match against the Go struct field names using https://pkg.go.dev/cloud.googles.ltd/go/internal/fields#List.Match, others decode methods should also follow the same

@rahul2393 rahul2393 requested review from a team as code owners February 13, 2024 14:39
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: spanner Issues related to the Spanner API. labels Feb 13, 2024
@rahul2393 rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 13, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 13, 2024
spanner/row_test.go Outdated Show resolved Hide resolved
@@ -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....

@rahul2393 rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 13, 2024
@rahul2393 rahul2393 merged commit 7ff5356 into main Feb 13, 2024
13 checks passed
@rahul2393 rahul2393 deleted the fix_spanner_select_all branch February 13, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API. kokoro:force-run Add this label to force Kokoro to re-run the tests. size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spanner: SelectAll does not use the same field matching rules as Row.ToStruct
3 participants