[
https://issues.apache.org/jira/browse/THRIFT-5300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17386690#comment-17386690
]
Jens Geyer edited comment on THRIFT-5300 at 7/24/21, 10:02 AM:
---------------------------------------------------------------
{quote}So my understanding is that the part in TCompactProtocol spec saying
that the field ids are different in set/list/map is wrong (except that booleans
need special handling, which is also implemented in the language libraries).
It's the same as the struct encoding in all (or at least most) implementations.
The fact that they all pass the cross language test also suggests that the
implementations are consistent.
{quote}
I think that we have to look at this from a different perspective.
I agree that the wording is probably a bit unfortunate.
Simply saying that "in general we have the exact same values, except for
booleans so it is the same" is too short-sighted of a view, and the reality is
already, today, that in fact we have two independent sets of IDs whose values
only +happen to match+ in a certain range.
Its about the *intention*, not about comparing two sets of numbers and declare
a degree of equality and postulate that what looks equal enough must therefore
by definition be "the same just with some exceptions". And the intention is
that compact explicitly reserves the right to use a different set of type IDs
if such adds technical value.
Hence, I would formulate the spec in a way, something like (DRAFT):
{quote}_Although the curent set of type IDs used is roughly equal to Thrift's
standard type IDs, one should not rely on that fact. Instead, it is recommended
to treat the compact type IDs as completely independent from the standard type
IDs._
{quote}
was (Author: jensg):
{quote}So my understanding is that the part in TCompactProtocol spec saying
that the field ids are different in set/list/map is wrong (except that booleans
need special handling, which is also implemented in the language libraries).
It's the same as the struct encoding in all (or at least most) implementations.
The fact that they all pass the cross language test also suggests that the
implementations are consistent.{quote}
I think that we have to look at this from a different perspective.
I agree that the wording is probably a bit unfortunate. As I understand it, the
intention was more something like that the IDs should be treated as an
independent set of IDs. Although they may more or less use the same values, it
is not a guaranteed fact that this will be always the case. We already have the
one case of booleans, and there may be more differences in the future, if there
are more data types to be added.
Simply saying that "in general we have the exact same values, except for
booleans so it is the same" is too short-sighted of a view, and the reality is
already, today, that in fact we have two independent sets of IDs whose values
only +happen to match+ in a certain range.
Its about the *intention*, not about comparing two sets of numbers and declare
a degree of equality and postulate that what looks equal enough must therefore
by definition be "the same just with some exceptions". And the intention is
that compact explicitly reserves the right to use a different set of type IDs
if such adds technical value.
Hence, I would formulate the spec in a way, something like (DRAFT):
bq. _Although the curent set of type IDs used is roughly equal to Thrift's
standard type IDs, one should not rely on that fact. Instead, it is recommended
to treat the compact type IDs as completely independent from the standard type
IDs._
> 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)
