Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-1039

Java codegen makes byte[] get/set for binary field, List<ByteBuffer> get/set for list<binary> field

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Java - Compiler
    • Labels:
    • Environment:

      All

      Description

      Since THRIFT-830, binary fields are implemented using ByteBuffer.

      struct A {
      1: binary bin_field,
      }

      will generate a method 'byte[] getBin_field()' which will optionally resize the underlying ByteBuffer so it has a capacity of 'remaining()' and return the backing array of this new ByteBuffer.

      This has several implications.

      First the ByteBuffer in 'bin_field' might be modified, thus misleading users into thinking that the call to 'getBin_field()' had no effect on the underlying structure.

      Thus the following will fail:

      byte[] b = new byte[2];
      b[0] = 0x01;
      A a = new A();
      a.setBin_field(ByteBuffer.wrap(b, 0, 1));
      byte[] bb = a.getBin_field();
      b[0] = 0x02;
      Assert.assertEquals(0x02, bb[0]);

      Second it creates a singularity in the getters as all other getters involving containers of binary data will return collections of ByteBuffer.

      I suggest we refactor to something more in the line of what HADOOP-6298 has done, i.e. provide a copyBytes() method which copies the bytes in a ByteBuffer and returns a correctly sized array.

      Such a method could be generated for each structure, with the following signature:

      public byte[] copyBytes(ByteBuffer b);

      This method would basically do what the current 'getBin_field' type methods do (allocate a byte array and fill it with the ByteBuffer's data), except it would not modify the structure's internal ByteBuffers.

        Attachments

        1. THRIFT-1039.patch
          6 kB
          Mathias Herberts

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              herberts Mathias Herberts
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated: