truss icon indicating copy to clipboard operation
truss copied to clipboard

not generating http_transport and client correctly for proto2

Open tpiecora opened this issue 6 years ago • 2 comments

When switching my proto file to syntax = "proto2", builds fail. Proto2 generates structs with pointers, while proto3 does not use pointers.

I get errors like these when trying to build:

# echo/echo-service/svc/client/cli/handlers
echo-service/svc/client/cli/handlers/handlers.go:15:5: cannot use IdGetEcho (type string) as type *string in field value

echo-service/svc/transport_http.go:164:9: cannot use IdGetEcho (type string) as type *string in assignment

The generated http transport and handler files do not seem to be handling the pointers correctly.

I am also getting an extra field generated called XXXUnrecognized on my message structs that I do not get with proto3.

The back story here is I would rather just use the proto3 syntax, but due to the lack of pointers it makes a bit of a hassle to have my generated protobuf structs work directly with database libraries due to the inability to handle null values. Being able to generate structs that can use sql.NullString, etc. would be nice, but at least in proto2 it would give you pointers that could take a nil. If there's a reasonable workaround for that problem using proto3 then I don't really care about the proto2 problems.

tpiecora avatar Oct 04 '17 03:10 tpiecora

@tpiecora In these cases I typically write a separate type to scan into or if its only a couple fields I define individual sql.NullString/etc fields to scan into. Then write some logic to get the logic into the pb type. Something like

type struct pbType {
    foo string
    bar int
}
thing := pbType{}
// some query
// select foo, bar from table // where foo can be null
var foo sql.NullString
row.Scan(&foo, &thing.bar)
thing.foo = foo.String

In practice this approach hasn't grown out of hand yet, but this use case could present a legitimate argument for generating sql structs if more interest is shown.

zaquestion avatar Oct 12 '17 23:10 zaquestion

For a one off case I think your solution is fine. For supporting null values (which it might be argued shouldn't be done) in general, a consistent and generator friendly way of doing so would be much preferred. Separating out the struct used for persistence from the transport would be ok I think if the maintenance of it could be minimized.

tpiecora avatar Oct 13 '17 11:10 tpiecora