thrift-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Boiles (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (THRIFT-4868) Go bug for optional sets with a default value
Date Fri, 17 May 2019 01:55:00 GMT

    [ https://issues.apache.org/jira/browse/THRIFT-4868?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16841848#comment-16841848
] 

John Boiles commented on THRIFT-4868:
-------------------------------------

For thoroughness sake, I did try the other combination of required with defaults and this
only happens for optional sets with a default value. For example 
{code}
struct SetPointerTest {
  1: required set<string> req_with_default = [ "test" ]
  2: required set<string> req_no_default
  3: optional set<string> opt_with_default = [ "test" ]
  4: optional set<string> opt_no_default
}
{code}
Gets generated as
{code}
type SetPointerTest struct {
  ReqWithDefault []string `thrift:"req_with_default,1,required" db:"req_with_default" json:"req_with_default"`
  ReqNoDefault []string `thrift:"req_no_default,2,required" db:"req_no_default" json:"req_no_default"`
  OptWithDefault *[]string `thrift:"opt_with_default,3" db:"opt_with_default" json:"opt_with_default"`
  OptNoDefault []string `thrift:"opt_no_default,4" db:"opt_no_default" json:"opt_no_default,omitempty"`
}
{code}

> Go bug for optional sets with a default value
> ---------------------------------------------
>
>                 Key: THRIFT-4868
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4868
>             Project: Thrift
>          Issue Type: Task
>          Components: Go - Compiler
>    Affects Versions: 0.12.0
>            Reporter: John Boiles
>            Priority: Major
>
> There's a bug in the Thrift Go generator that happens when optional set types are used
with a default field. For example, a simple struct like this:
> {code:java}
> struct SetPointerTest {
>   1: optional set<string> with_default = [ "test" ]
> }
> {code}
> Generates a struct like this
> {code}
> type SetPointerTest struct {
>   WithDefault *[]string `thrift:"with_default,1" db:"with_default" json:"with_default"`
> }
> {code}
> And associated Go code with a line like this:
> {code:go}
> if reflect.DeepEqual(*p.WithDefault[i],*p.WithDefault[j]) {
> {code}
> Which fails with an error
> {code}
> gen-go/setpointertest/SetPointerTest.go:125:44: invalid operation: p.WithDefault[i] (type
*[]string does not support indexing)
> {code}
> Thrift optional sets only get translated to Go pointer values (causing this error) when
a default value is set. This is because the {{t_go_generator::is_pointer_field}} method has
[some logic|https://github.com/apache/thrift/blob/master/compiler/cpp/src/thrift/generate/t_go_generator.cc#L410-L411]
that returns {{true}} if the set has a default value. I'm unsure why this is the case; it
seems simpler to not change the type when there's a default value. A Golang slice is a pointer
value by default, so {{nil}} can still be used for {{optional}} fields. I'd be super interested
to understand why the code was written this way.
> However, since changing the pointer behavior would be a version-incompatible change,
I propose we do the simple thing for now to fix Go compilation. I'll submit a patch with a
test shortly.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message