[
https://issues.apache.org/jira/browse/THRIFT-3752?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15208762#comment-15208762
]
John Sirois edited comment on THRIFT-3752 at 3/23/16 4:55 PM:
--------------------------------------------------------------
Hrm, so this may not be a thrift compiler bug per-se. Marking
{{TaskQuery.taskIds}} as optional yields:
{noformat}
type TaskQuery struct {
TaskIds map[string]bool `thrift:"taskIds,4"
json:"taskIds,omitempty"`
}
func (p *TaskQuery) IsSetTaskIds() bool {
return p.TaskIds != nil
}
func (p *TaskQuery) writeField4(oprot thrift.TProtocol) (err error) {
if p.IsSetTaskIds() {
if err := oprot.WriteFieldBegin("taskIds", thrift.SET, 4); err != nil {
return thrift.PrependError(fmt.Sprintf("%T write field begin error
4:taskIds: ", p), err)
}
if err := oprot.WriteSetBegin(thrift.STRING, len(p.TaskIds)); err !=
nil {
return thrift.PrependError("error writing set begin: ", err)
}
for v, _ := range p.TaskIds {
if err := oprot.WriteString(string(v)); err != nil {
return thrift.PrependError(fmt.Sprintf("%T. (0) field write
error: ", p), err)
}
}
if err := oprot.WriteSetEnd(); err != nil {
return thrift.PrependError("error writing set end: ", err)
}
if err := oprot.WriteFieldEnd(); err != nil {
return thrift.PrependError(fmt.Sprintf("%T write field end error
4:taskIds: ", p), err)
}
}
return err
}
{noformat}
The {{IsSetTaskIds}} method is new and its used to skip serialization for an
unset {{taskIds}} field is what we want.
In other words, the thirft compiler for Go has a different notion of unset
requiredness than the java compiler (The TaskQuery receiver in this case is the
Aurora scheduler - written in java). The perils of thrift's global lack of
specificity as to how to handle optional vs required vs <none> is well
documented and the real issue here :/.
[~jfarrell] Has there been any discussion of fixing this, declaring bankruptcy,
or picking some other path to deal with incompatible behavior across the
various language backends wrt requiredness?
was (Author: jsirois):
Hrm, so this may not be a thrift compiler bug per-se. Marking
{{TaskQuery.taskIds}} as optional yields:
{noformat}
type TaskQuery struct {
TaskIds map[string]bool `thrift:"taskIds,4"
json:"taskIds,omitempty"`
}
func (p *TaskQuery) IsSetTaskIds() bool {
return p.TaskIds != nil
}
func (p *TaskQuery) writeField4(oprot thrift.TProtocol) (err error) {
if p.IsSetTaskIds() {
if err := oprot.WriteFieldBegin("taskIds", thrift.SET, 4); err != nil {
return thrift.PrependError(fmt.Sprintf("%T write field begin error
4:taskIds: ", p), err)
}
if err := oprot.WriteSetBegin(thrift.STRING, len(p.TaskIds)); err !=
nil {
return thrift.PrependError("error writing set begin: ", err)
}
for v, _ := range p.TaskIds {
if err := oprot.WriteString(string(v)); err != nil {
return thrift.PrependError(fmt.Sprintf("%T. (0) field write
error: ", p), err)
}
}
if err := oprot.WriteSetEnd(); err != nil {
return thrift.PrependError("error writing set end: ", err)
}
if err := oprot.WriteFieldEnd(); err != nil {
return thrift.PrependError(fmt.Sprintf("%T write field end error
4:taskIds: ", p), err)
}
}
return err
}
{noformat}
The {{IsSetTaskIds}} method is new and its use to skip serialization for an
unset {{taskIds}} field is what we want.
In other words, the thirft compiler for Go has a different notion of unset
requiredness than the java compiler (The TaskQuery receiver in this case is the
Aurora scheduler - written in java). The perils of thrift's global lack of
specificity as to how to handle optional vs required vs <none> is well
documented and the real issue here :/.
[~jfarrell] Has there been any discussion of fixing this, declaring bankruptcy,
or picking some other path to deal with incompatible behavior across the
various language backends wrt requiredness?
> nil collections are serialized as empty collections
> ---------------------------------------------------
>
> Key: THRIFT-3752
> URL: https://issues.apache.org/jira/browse/THRIFT-3752
> Project: Thrift
> Issue Type: Bug
> Components: Go - Compiler
> Reporter: John Sirois
> Assignee: John Sirois
>
> See discussion here: https://reviews.apache.org/r/45193/
> This is likely related to THRIFT-3700.
> In short, for this struct:
> {noformat}
> struct TaskQuery {
> 4: optional set<string> taskIds
> }
> {noformat}
> The following go struct is generated:
> {noformat}
> type TaskQuery struct {
> TaskIds map[string]bool `thrift:"taskIds,4" json:"taskIds"`
> }
> {noformat}
> This is all well and good, since {{TaskQuery{}.TaskIds == nil}}; ie the
> {{TaskIds}} collection field is - by default - unset.
> The problem is in the serialization for the field, which wipes out the unset
> ({{nil}}) vs empty collection distinction at the wire level:
> {noformat}
> func (p *TaskQuery) writeField4(oprot thrift.TProtocol) (err error) {
> if err := oprot.WriteFieldBegin("taskIds", thrift.SET, 4); err != nil {
> return thrift.PrependError(fmt.Sprintf("%T write field begin
> error 4:taskIds: ", p), err)
> }
> if err := oprot.WriteSetBegin(thrift.STRING, len(p.TaskIds)); err !=
> nil {
> return thrift.PrependError("error writing set begin: ", err)
> }
> for v, _ := range p.TaskIds {
> if err := oprot.WriteString(string(v)); err != nil {
> return thrift.PrependError(fmt.Sprintf("%T. (0) field
> write error: ", p), err)
> }
> }
> if err := oprot.WriteSetEnd(); err != nil {
> return thrift.PrependError("error writing set end: ", err)
> }
> if err := oprot.WriteFieldEnd(); err != nil {
> return thrift.PrependError(fmt.Sprintf("%T write field end
> error 4:taskIds: ", p), err)
> }
> return err
> }
> {noformat}
> So on the receiving end of the wire, a {{nil}} collection is turned into an
> empty collection and so unset-ness cannot be distinguished from set but empty.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
