HBase
  1. HBase
  2. HBASE-4648

Bytes.toBigDecimal() doesn't use offset

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.90.4
    • Fix Version/s: 0.92.0, 0.94.0
    • Component/s: util
    • Labels:
      None
    • Environment:

      Java 1.6.0_26, Mac OS X 10.7 and CentOS 6

    • Hadoop Flags:
      Incompatible change, Reviewed

      Description

      The Bytes.toBigDecimal(byte[], offset, len) method does not use the offset, thus you will get an incorrect result for the BigDecimal unless the BigDecimal's bytes are at the beginning of the byte array.

      1. bigdec.patch
        4 kB
        Bryan Keller
      2. bigdec2.patch
        4 kB
        Bryan Keller

        Activity

        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2371 (See https://builds.apache.org/job/HBase-TRUNK/2371/)
        HBASE-4648 Bytes.toBigDecimal() doesn't use offset

        larsh :
        Files :

        • /hbase/trunk/CHANGES.txt
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/Bytes.java
        • /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestBytes.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2371 (See https://builds.apache.org/job/HBase-TRUNK/2371/ ) HBASE-4648 Bytes.toBigDecimal() doesn't use offset larsh : Files : /hbase/trunk/CHANGES.txt /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/Bytes.java /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestBytes.java
        Hide
        stack added a comment -

        +1 on patch and +1 on 0.92 and TRUNK only.

        Show
        stack added a comment - +1 on patch and +1 on 0.92 and TRUNK only.
        Hide
        Hudson added a comment -

        Integrated in HBase-0.92 #82 (See https://builds.apache.org/job/HBase-0.92/82/)
        HBASE-4648 Bytes.toBigDecimal() doesn't use offset

        larsh :
        Files :

        • /hbase/branches/0.92/CHANGES.txt
        • /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/util/Bytes.java
        • /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/util/TestBytes.java
        Show
        Hudson added a comment - Integrated in HBase-0.92 #82 (See https://builds.apache.org/job/HBase-0.92/82/ ) HBASE-4648 Bytes.toBigDecimal() doesn't use offset larsh : Files : /hbase/branches/0.92/CHANGES.txt /hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/util/Bytes.java /hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/util/TestBytes.java
        Hide
        Lars Hofhansl added a comment -

        Thanks for the patch Bryan. (and I just realized I misspelled your name in the commit log)

        Show
        Lars Hofhansl added a comment - Thanks for the patch Bryan. (and I just realized I misspelled your name in the commit log)
        Hide
        Lars Hofhansl added a comment -

        Committed to 0.92 and trunk. Since this is a (slightly) incompatible change I did not commit it to 0.90... Open again if you feel otherwise.

        Show
        Lars Hofhansl added a comment - Committed to 0.92 and trunk. Since this is a (slightly) incompatible change I did not commit it to 0.90... Open again if you feel otherwise.
        Hide
        Lars Hofhansl added a comment - - edited

        Patch looks good to me.
        I'm fine applying to 0.90, 0.92, and trunk. Nobody should use toBigDecimal(byte[], int).

        If nobody objects I'll commit this later today.

        Show
        Lars Hofhansl added a comment - - edited Patch looks good to me. I'm fine applying to 0.90, 0.92, and trunk. Nobody should use toBigDecimal(byte[], int). If nobody objects I'll commit this later today.
        Hide
        Bryan Keller added a comment -

        Sorry bad patch, here's a new one.

        Show
        Bryan Keller added a comment - Sorry bad patch, here's a new one.
        Hide
        Bryan Keller added a comment -

        Here is the patch. I also added some tests for testing serialization with offsets for other data types.

        Show
        Bryan Keller added a comment - Here is the patch. I also added some tests for testing serialization with offsets for other data types.
        Hide
        Bryan Keller added a comment -

        I was actually thinking that method isn't useful, it seems it should be removed, as the BigDecimal serialization is variable-length. It would only be used if the BigDecimal comes at the end of the byte array.

        Show
        Bryan Keller added a comment - I was actually thinking that method isn't useful, it seems it should be removed, as the BigDecimal serialization is variable-length. It would only be used if the BigDecimal comes at the end of the byte array.
        Hide
        Lars Hofhansl added a comment -

        This:

          public static BigDecimal toBigDecimal(byte[] bytes, int offset) {
            return toBigDecimal(bytes, offset, bytes.length);
          }
        

        Is also wrong. Should be:

            return toBigDecimal(bytes, offset, bytes.length-offset);
        
        Show
        Lars Hofhansl added a comment - This: public static BigDecimal toBigDecimal( byte [] bytes, int offset) { return toBigDecimal(bytes, offset, bytes.length); } Is also wrong. Should be: return toBigDecimal(bytes, offset, bytes.length-offset);
        Hide
        Bryan Keller added a comment -

        I'll upload the patch a little later today, sorry for the delay.

        Show
        Bryan Keller added a comment - I'll upload the patch a little later today, sorry for the delay.
        Hide
        Lars Hofhansl added a comment -

        Whoa... Good find Bryan.

        Show
        Lars Hofhansl added a comment - Whoa... Good find Bryan.
        Hide
        stack added a comment -

        You have a patch please Bryan?

        Show
        stack added a comment - You have a patch please Bryan?

          People

          • Assignee:
            Unassigned
            Reporter:
            Bryan Keller
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development