Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-1920 Binary Type
  3. THRIFT-429

Make binary a full-fledged type of its own

    XMLWordPrintableJSON

    Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Won't Fix
    • Affects Version/s: 0.1
    • Fix Version/s: None
    • Component/s: Compiler (General)
    • Labels:
      None

      Description

      From the point of view of the protocol abstraction, the 'binary' type looks like a full-fledged type but under the covers the implementation is such that the wire format encodes 'string' and 'binary' types with the same type code. I'd like to see 'binary' become a full-fledged type instead.

      This wart of the implementation has at least one strange and unexpected implication: protocol implementations must be careful that 'string' values can be skipped properly by way of the readBinary() method. This affects protocols where the wire format of 'string' is different from the wire format of 'binary'. I encountered this problem when implementing the JSON protocol.

      I am not sure if there are other implications of this wart – it's possible there are others that have not been surfaced yet.

      This is unfortunately a backwards-compatibility breaking change and has implications for those who have large investments in persisted Thrift structures. I realize that the down-side here so far is fairly limited – the JSON protocol was able to work around this without too big a problem. We had discussed this a year ago and got some level of agreement that we wouldn't do this until there were some other significant backwards-compatibility breakage.

      However, it only becomes harder over time for such a change to be made, as more people persist more and more Thrift data. I think it is pretty clear that in retrospect we would not have designed things such that the same type code is used on the wire for 'string' and 'binary'. I wonder if the current users of Thrift who would be impacted by this now would agree to take on the pain of this change now so that future users will not have to go through this pain (also, their own pain might be lessened by doing this sooner rather than later).

      I am raising the issue and targeting for 0.1 to prompt a little bit of discussion.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                jking3 James E. King III
                Reporter:
                cwalters Chad Walters
              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: