[
https://issues.apache.org/jira/browse/THRIFT-5300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17225130#comment-17225130
]
Allen George edited comment on THRIFT-5300 at 11/3/20, 5:00 AM:
----------------------------------------------------------------
[~shuoli84] This is what I get for trying to squeeze in looking at this ticket
in the middle of other tasks.
I apologize for wasting everyone's time here. I'm stepping back and trying to
state what I've discovered about the spec, and implementations - and hopefully
people can point out where I'm mistaken and missing things.
The issue at hand appears to be a mismatch between the spec, which states that
*in the compact protocol* the type encoded in a struct has a different value
than the type encoded in collections (sets, lists, maps). I've attached
screenshots of the spec and you've referenced it in the description.
I've tried to see what multiple implementations do:
* The rust implementation uses the same type encoding for both compact
structs/collections.
* The golang implementation uses the same type encoding for both compact
structs/collections.
* The Java implementation uses the same type encoding for both compact
structs/collections.
* The Python implementation uses the same type encoding for both compact
structs/collections.
* The Swift implementation uses the same type encoding for both compact
structs/collections. (AFAICT)
* The cpp implementation *appears to follow the spec*.
* The c_glib implementation *appears to follow the spec*.
Before I proceed, can you confirm that the above is your understanding as well?
EDIT:
Rereading the Java implementation closer it appears to follow the spec as well?
If that's the case, I'm starting to understand why this has been so confusing.
was (Author: allengeorge):
[~shuoli84] This is what I get for trying to squeeze in looking at this ticket
in the middle of other tasks.
I apologize for wasting everyone's time here. I'm stepping back and trying to
state what I've discovered about the spec, and implementations - and hopefully
people can point out where I'm mistaken and missing things.
The issue at hand appears to be a mismatch between the spec, which states that
*in the compact protocol* the type encoded in a struct has a different value
than the type encoded in collections (sets, lists, maps). I've attached
screenshots of the spec and you've referenced it in the description.
I've tried to see what multiple implementations do:
* The rust implementation uses the same type encoding for both compact
structs/collections.
* The golang implementation uses the same type encoding for both compact
structs/collections.
* The Java implementation uses the same type encoding for both compact
structs/collections.
* The Python implementation uses the same type encoding for both compact
structs/collections.
* The Swift implementation uses the same type encoding for both compact
structs/collections. (AFAICT)
* The cpp implementation *appears to follow the spec*.
* The c_glib implementation *appears to follow the spec*.
Before I proceed, can you confirm that the above is your understanding as well?
EDIT:
Rereading the Java spec closer it appears to follow the spec as well? If that's
the case, I'm starting to understand why this has been so confusing.
> rs compact protocol collection elem type to ttype mapping wrong
> ---------------------------------------------------------------
>
> Key: THRIFT-5300
> URL: https://issues.apache.org/jira/browse/THRIFT-5300
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Library
> Affects Versions: 0.13.0
> Reporter: shuo li
> Assignee: Allen George
> Priority: Major
> Attachments: Screen Shot 2020-11-02 at 16.31.09.png, Screen Shot
> 2020-11-02 at 16.31.31.png
>
>
> collection_u8_to_type only overrides bool, but in spec, other types are
> different too.
> In field:
> * {{BOOLEAN_TRUE}}, encoded as {{1}}
> * {{BOOLEAN_FALSE}}, encoded as {{2}}
> * {{BYTE}}, encoded as {{3}}
> * {color:#FF0000}{{I16}}, encoded as {{4}}{color}
> * {color:#FF0000}{{I32}}, encoded as {{5}}{color}
> * {{I64}}, encoded as {{6}}
> * {{DOUBLE}}, encoded as {{7}}
> * {{BINARY}}, used for binary and string fields, encoded as {{8}}
> * {{LIST}}, encoded as {{9}}
> * {{SET}}, encoded as {{10}}
> * {{MAP}}, encoded as {{11}}
> * {{STRUCT}}, used for both structs and union fields, encoded as {{12}}
> In colleciton:
> * {{BOOL}}, encoded as {{2}}
> * {{BYTE}}, encoded as {{3}}
> * {{DOUBLE}}, encoded as {{4}}
> * {color:#FF0000}{{I16}}, encoded as {{6}}{color}
> * {color:#FF0000}{{I32}}, encoded as {{8}}{color}
> * {{I64}}, encoded as {{10}}
> * {{STRING}}, used for binary and string fields, encoded as {{11}}
> * {{STRUCT}}, used for structs and union fields, encoded as {{12}}
> * {{MAP}}, encoded as {{13}}
> * {{SET}}, encoded as {{14}}
> * {{LIST}}, encoded as {{15}}
> {code:java}
> // code placeholder
> fn collection_u8_to_type(b: u8) -> crate::Result<TType> {
> match b {
> 0x01 => Ok(TType::Bool),
> o => u8_to_type(o),
> }
> }
> fn u8_to_type(b: u8) -> crate::Result<TType> {
> match b {
> 0x00 => Ok(TType::Stop),
> 0x03 => Ok(TType::I08), // equivalent to TType::Byte
> 0x04 => Ok(TType::I16),
> 0x05 => Ok(TType::I32),
> 0x06 => Ok(TType::I64),
> 0x07 => Ok(TType::Double),
> 0x08 => Ok(TType::String),
> 0x09 => Ok(TType::List),
> 0x0A => Ok(TType::Set),
> 0x0B => Ok(TType::Map),
> 0x0C => Ok(TType::Struct),
> unkn => Err(crate::Error::Protocol(crate::ProtocolError {
> kind: crate::ProtocolErrorKind::InvalidData, message:
> format!("cannot convert {} into TType", unkn),
> })),
> }}
> {code}
>
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
