-
Notifications
You must be signed in to change notification settings - Fork 1.5k
spanner: SelectAll does not correctly handle structs with spanner tag annotations that contain uppercase letters #9459
Description
Client
Spanner
Environment
Get this on all, tested on local mac PC with Docker
Go Environment
$ go version
go version go1.21.7 darwin/arm64
$ go env
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/{masked}/.asdf/installs/golang/1.21.7/packages/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/{masked}/.asdf/installs/golang/1.21.7/packages'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/{masked}/.asdf/installs/golang/1.21.7/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/{masked}/.asdf/installs/golang/1.21.7/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.21.7'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/{masked}/Projects/yukia3e/go-spanner-selectall-test/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/zc/ljr5xzz11mn7dflgz3ghw_tm0000gn/T/go-build3650580967=/tmp/go-build -gno-record-gcc-switches -fno-common'
Code
I have created a sample repository where you can reproduce the issue.
https://github.com/yukia3e/go-spanner-selectall-test/blob/main/internal/main.go#L115
The following fixes were incorporated in v1.57.0, but I think that the behavior of spanner:"" tag annotation still seems different from that of ToStruct.
spanner: SelectAll struct fields match should be case-insensitive (#9417) (7ff5356)
The current behavior of SelectAll for spanner tag annotations seems to be "accept only lowercase field names" instead of case-insensitive.
First, suppose I have the following test table
CREATE TABLE Singers (
ArtistId INT64 NOT NULL, Name
Name STRING(1024), Name
) PRIMARY KEY (ArtistId)Also, the following data is submitted as test data.
INSERT INTO Singers (ArtistId, Name) VALUES (1, 'Alice'), (2, 'Bob'), (3, 'Charlie');
Consider the case where the *spanner.RowIterator value obtained from the above table is set to a slice of a struct containing uppercase letters in the spanner tag annotation as follows.
type Singer struct {
ID int64 `spanner: "ArtistId"`
Name string `spanner: "Name"`
}
Expected behavior
The expected result is that SelectAll will set the value correctly, as well as ToStruct, even if the tag annotation contains uppercase letters.
Actual behavior
An error or panic will occur if the tag annotation contains uppercase letters. If spanner.WithLenient() option is not specified, an error will occur; if spanner.WithLenient() option is specified, panic will occur.
- Error message if
spanner.WithLenient()is not specified
Go struct {ID:0 Name:}(type reflect.Value) has no or duplicate fields for Cloud Spanner STRUCT field ArtistId
- If
spanner.WithLenient()is specified.
panic: reflect.Set: value of type string is not assignable to type int64
Additional context
I have investigated the possible cause of the problem.
In row.go, initFieldTag L564
name, keep, _, _ := spannerTagParser(fieldType.Tag)
if !keep {
continue
}
if name == "" {
name = strings.ToLower(fieldType.Name)
}
(*fieldTagMap)[name] = sliceItem.Field(i)but this is probably because strings.ToLower is not applied to the spannerTagParser result.
I have tested the above test repository with the following changes to the row.go implementation and have confirmed that it works correctly.
name, keep, _, _ := spannerTagParser(fieldType.Tag)
if !keep {
continue
}
if name == "" {
name = fieldType.Name
}
(*fieldTagMap)[strings.ToLower(name)] = sliceItem.Field(i)As this is my first contribution to this repository, I am not fully aware of the detailed testing procedures, but I will be able to issue a PR to correct the relevant areas.
The branch where we made the modifications is here.