Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.98.1, 0.99.0
    • Component/s: None
    • Hadoop Flags:
      Reviewed

      Description

      DoubleColumnInterpreter was requested by Royston Sellman.

      1. HBase.proto
        5 kB
        Julian Wissmann
      2. TestDoubleColumnInterpreter.java
        23 kB
        Julian Wissmann
      3. DoubleColumnInterpreter.java
        3 kB
        Julian Wissmann
      4. DoubleColumnInterpreter.patch
        3.54 MB
        Julian Wissmann
      5. DoubleColumnInterpreterV2.patch
        1.21 MB
        Julian Wissmann
      6. DoubleColumnInterpreterV3.patch
        52 kB
        Julian Wissmann
      7. DoubleColumnInterpreterV4.patch
        52 kB
        Julian Wissmann
      8. 5175-v5.txt
        52 kB
        Ted Yu

        Activity

        Hide
        Enis Soztutar added a comment -

        Closing this issue after 0.99.0 release.

        Show
        Enis Soztutar added a comment - Closing this issue after 0.99.0 release.
        Hide
        Hudson added a comment -

        ABORTED: Integrated in HBase-TRUNK #5038 (See https://builds.apache.org/job/HBase-TRUNK/5038/)
        HBASE-5175 Add DoubleColumnInterpreter (Julian Wissmann) (tedyu: rev 1581200)

        • /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/coprocessor/DoubleColumnInterpreter.java
        • /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java
        • /hbase/trunk/hbase-protocol/src/main/protobuf/HBase.proto
        • /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestDoubleColumnInterpreter.java
        Show
        Hudson added a comment - ABORTED: Integrated in HBase-TRUNK #5038 (See https://builds.apache.org/job/HBase-TRUNK/5038/ ) HBASE-5175 Add DoubleColumnInterpreter (Julian Wissmann) (tedyu: rev 1581200) /hbase/trunk/hbase-client/src/main/java/org/apache/hadoop/hbase/client/coprocessor/DoubleColumnInterpreter.java /hbase/trunk/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java /hbase/trunk/hbase-protocol/src/main/protobuf/HBase.proto /hbase/trunk/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestDoubleColumnInterpreter.java
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in HBase-0.98 #250 (See https://builds.apache.org/job/HBase-0.98/250/)
        HBASE-5175 Add DoubleColumnInterpreter (Julian Wissmann) (tedyu: rev 1581199)

        • /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/coprocessor/DoubleColumnInterpreter.java
        • /hbase/branches/0.98/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java
        • /hbase/branches/0.98/hbase-protocol/src/main/protobuf/HBase.proto
        • /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestDoubleColumnInterpreter.java
        Show
        Hudson added a comment - SUCCESS: Integrated in HBase-0.98 #250 (See https://builds.apache.org/job/HBase-0.98/250/ ) HBASE-5175 Add DoubleColumnInterpreter (Julian Wissmann) (tedyu: rev 1581199) /hbase/branches/0.98/hbase-client/src/main/java/org/apache/hadoop/hbase/client/coprocessor/DoubleColumnInterpreter.java /hbase/branches/0.98/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java /hbase/branches/0.98/hbase-protocol/src/main/protobuf/HBase.proto /hbase/branches/0.98/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestDoubleColumnInterpreter.java
        Hide
        Ted Yu added a comment -

        Logged HBASE-10824 for line length check enhancement.

        Show
        Ted Yu added a comment - Logged HBASE-10824 for line length check enhancement.
        Hide
        Ted Yu added a comment -

        Here is the filter for protobuf generated code:

          lines=`cat $PATCH_DIR/patch | grep "^+" | grep -v "^@@" | grep -v "^+++" | grep -v "import" | grep -v "hbase.protobuf.generated" | awk -v len="$MAX_LINE_LENGTH_PATCH"    'length ($0) > len' | head -n 10`
        

        Obviously 'com.google.protobuf.' should be considered as well.
        Let me log another JIRA for enhancement.

        Show
        Ted Yu added a comment - Here is the filter for protobuf generated code: lines=`cat $PATCH_DIR/patch | grep "^+" | grep -v "^@@" | grep -v "^+++" | grep -v " import " | grep -v "hbase.protobuf.generated" | awk -v len= "$MAX_LINE_LENGTH_PATCH" 'length ($0) > len' | head -n 10` Obviously 'com.google.protobuf.' should be considered as well. Let me log another JIRA for enhancement.
        Hide
        Anoop Sam John added a comment -

        The long lines were from protobuf generated code.

        !! We have excluded protobuf generated code from this line length checks. This is broken?

        Show
        Anoop Sam John added a comment - The long lines were from protobuf generated code. !! We have excluded protobuf generated code from this line length checks. This is broken?
        Hide
        Ted Yu added a comment -

        The long lines were from protobuf generated code.

        Show
        Ted Yu added a comment - The long lines were from protobuf generated code.
        Hide
        Anoop Sam John added a comment -

        +1
        Pls fix the line length issue on commit.

        Show
        Anoop Sam John added a comment - +1 Pls fix the line length issue on commit.
        Hide
        Honghua Feng added a comment -

        +1

        Show
        Honghua Feng added a comment - +1
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12636453/5175-v5.txt
        against trunk revision .
        ATTACHMENT ID: 12636453

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 3 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 lineLengths. The patch introduces the following lines longer than 100:
        + private DoubleMsg(boolean noInit)

        { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); }

        + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg()));

        +1 site. The mvn site goal succeeds with this patch.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12636453/5175-v5.txt against trunk revision . ATTACHMENT ID: 12636453 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 3 new or modified tests. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + private DoubleMsg(boolean noInit) { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); } + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg())); +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9082//console This message is automatically generated.
        Hide
        Julian Wissmann added a comment -

        I was just re-running relevant tests. Looks like you were faster. Thanks for finishing it up.

        Show
        Julian Wissmann added a comment - I was just re-running relevant tests. Looks like you were faster. Thanks for finishing it up.
        Hide
        Ted Yu added a comment -

        Patch v5 addresses Anoop's comments.

        Show
        Ted Yu added a comment - Patch v5 addresses Anoop's comments.
        Hide
        Anoop Sam John added a comment -

        Overall looks good.
        Instead of the deprecated method can you implement public T getValue(byte[] colFamily, byte[] colQualifier, Cell c). No diff in impl code needed.

        +   if (d1 == null ^ d2 == null) {
        +      return (d1 == null) ? d2 : d1; // either of one is null.
        +    } 
        +    if (d1 == null) {// both are null
        +      return null;
        +    }
        

        This would be enough I guess.

        +    if (d1 == null || d2 == null) {
        +      return (d1 == null) ? d2 : d1; 
        +    }
        
        Show
        Anoop Sam John added a comment - Overall looks good. Instead of the deprecated method can you implement public T getValue(byte[] colFamily, byte[] colQualifier, Cell c). No diff in impl code needed. + if (d1 == null ^ d2 == null ) { + return (d1 == null ) ? d2 : d1; // either of one is null . + } + if (d1 == null ) { // both are null + return null ; + } This would be enough I guess. + if (d1 == null || d2 == null ) { + return (d1 == null ) ? d2 : d1; + }
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12636009/DoubleColumnInterpreterV4.patch
        against trunk revision .
        ATTACHMENT ID: 12636009

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 2 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 lineLengths. The patch introduces the following lines longer than 100:
        + private DoubleMsg(boolean noInit)

        { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); }

        + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg()));

        +1 site. The mvn site goal succeeds with this patch.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12636009/DoubleColumnInterpreterV4.patch against trunk revision . ATTACHMENT ID: 12636009 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + private DoubleMsg(boolean noInit) { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); } + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg())); +1 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9063//console This message is automatically generated.
        Hide
        Julian Wissmann added a comment -

        Sorry about the delay!
        New version of patch, fixing most issues.
        However I did not touch the Test timeouts as I assume 300000 to be a sane default value since it is used in TestAggregateProtocol and TestBigDecimalColumnInterpreter.

        Show
        Julian Wissmann added a comment - Sorry about the delay! New version of patch, fixing most issues. However I did not touch the Test timeouts as I assume 300000 to be a sane default value since it is used in TestAggregateProtocol and TestBigDecimalColumnInterpreter.
        Hide
        Ted Yu added a comment - - edited

        Some nits:

        +   @Override
        +  public Double add(Double d1, Double d2) {
        

        @Override should align with the start of 'p' in public.

        +    } else if (d1 == null) // both are null
        +      return null;
        

        'else' is not needed. Please enclose 'return null;' in braces.

        Running org.apache.hadoop.hbase.coprocessor.TestDoubleColumnInterpreter
        Tests run: 38, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 25.34 sec
        

        Good, new test passed.

        +  @Test(timeout = 300000)
        +  public void testMaxWithValidRange2WithNoCQ() throws Throwable {
        

        We don't need such long timeout, right ?

        +    } catch (Throwable e) {
        +    }
        

        Why is Throwable ignored ?
        The long line warnings were for protobuf generated code.

        Show
        Ted Yu added a comment - - edited Some nits: + @Override + public Double add( Double d1, Double d2) { @Override should align with the start of 'p' in public. + } else if (d1 == null ) // both are null + return null ; 'else' is not needed. Please enclose 'return null;' in braces. Running org.apache.hadoop.hbase.coprocessor.TestDoubleColumnInterpreter Tests run: 38, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 25.34 sec Good, new test passed. + @Test(timeout = 300000) + public void testMaxWithValidRange2WithNoCQ() throws Throwable { We don't need such long timeout, right ? + } catch (Throwable e) { + } Why is Throwable ignored ? The long line warnings were for protobuf generated code.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12634930/DoubleColumnInterpreterV3.patch
        against trunk revision .
        ATTACHMENT ID: 12634930

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 2 new or modified tests.

        +1 hadoop1.0. The patch compiles against the hadoop 1.0 profile.

        +1 hadoop1.1. The patch compiles against the hadoop 1.1 profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 lineLengths. The patch introduces the following lines longer than 100:
        + private DoubleMsg(boolean noInit)

        { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); }

        + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg()));

        +1 site. The mvn site goal succeeds with this patch.

        -1 core tests. The patch failed these unit tests:

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12634930/DoubleColumnInterpreterV3.patch against trunk revision . ATTACHMENT ID: 12634930 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 2 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop1.1 . The patch compiles against the hadoop 1.1 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. -1 lineLengths . The patch introduces the following lines longer than 100: + private DoubleMsg(boolean noInit) { this.unknownFields = com.google.protobuf.UnknownFieldSet.getDefaultInstance(); } + result = result && (Double.doubleToLongBits(getDoubleMsg()) == Double.doubleToLongBits(other.getDoubleMsg())); +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9016//console This message is automatically generated.
        Hide
        Julian Wissmann added a comment -

        Turns out using git to create a patch that svn will swallow isn't a no-brainer.
        My local system accepted it, this time, so I'm confident Jenkins will, too.

        Show
        Julian Wissmann added a comment - Turns out using git to create a patch that svn will swallow isn't a no-brainer. My local system accepted it, this time, so I'm confident Jenkins will, too.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12634745/DoubleColumnInterpreterV2.patch
        against trunk revision .
        ATTACHMENT ID: 12634745

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 4 new or modified tests.

        -1 patch. The patch command could not apply the patch.

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8998//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12634745/DoubleColumnInterpreterV2.patch against trunk revision . ATTACHMENT ID: 12634745 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 4 new or modified tests. -1 patch . The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/8998//console This message is automatically generated.
        Hide
        Julian Wissmann added a comment -

        Re-Created patch.
        Issues of the previous patch should be fixed.

        Show
        Julian Wissmann added a comment - Re-Created patch. Issues of the previous patch should be fixed.
        Hide
        Ted Yu added a comment -

        Diff for HBaseProtos.java seems to be included in the patch more than once:

        From 0b07009f891a73bd9e335bf925d5dfc7054ac8a9 Mon Sep 17 00:00:00 2001
        From: Julian Wissmann <julianwissmann@gmail.com>
        Date: Thu, 13 Mar 2014 13:05:07 +0100
        Subject: [PATCH 2/7] formatted protos
        
        ---
         .../hbase/protobuf/generated/HBaseProtos.java      | 7899 +++++++++++---------
         hbase-protocol/src/main/protobuf/HBase.proto       |    3 +
         2 files changed, 4517 insertions(+), 3385 deletions(-)
        
        diff --git a/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java b/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java
        

        There is no formatting needed, see:

        -// Generated by the protocol buffer compiler.  DO NOT EDIT!
        
        Show
        Ted Yu added a comment - Diff for HBaseProtos.java seems to be included in the patch more than once: From 0b07009f891a73bd9e335bf925d5dfc7054ac8a9 Mon Sep 17 00:00:00 2001 From: Julian Wissmann <julianwissmann@gmail.com> Date: Thu, 13 Mar 2014 13:05:07 +0100 Subject: [PATCH 2/7] formatted protos --- .../hbase/protobuf/generated/HBaseProtos.java | 7899 +++++++++++--------- hbase-protocol/src/main/protobuf/HBase.proto | 3 + 2 files changed, 4517 insertions(+), 3385 deletions(-) diff --git a/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java b/hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java There is no formatting needed, see: - // Generated by the protocol buffer compiler. DO NOT EDIT!
        Hide
        Julian Wissmann added a comment -

        Patch created

        -Wrapped long lines
        -Fixed Comment in DoubleColumnInterpreter to reference correct test
        -Added License Headers

        Are there any guidelines on formatting Protobuf autogenerated code, correctly? I autoindented that causing the patch to grow unnecessarily big.

        Show
        Julian Wissmann added a comment - Patch created -Wrapped long lines -Fixed Comment in DoubleColumnInterpreter to reference correct test -Added License Headers Are there any guidelines on formatting Protobuf autogenerated code, correctly? I autoindented that causing the patch to grow unnecessarily big.
        Hide
        Ted Yu added a comment -

        For DoubleColumnInterpreter.java :
        please add license header

         * is supposed to instantiate it and passed along as a parameter. See
         * TestAggregateProtocol methods for its sample usage.
        

        You're going to modify TestAggregateProtocol in next patch, right ?

        public class DoubleColumnInterpreter extends ColumnInterpreter<Double, Double, EmptyMsg, DoubleMsg, DoubleMsg>{
        

        Wrap long line.

        Show
        Ted Yu added a comment - For DoubleColumnInterpreter.java : please add license header * is supposed to instantiate it and passed along as a parameter. See * TestAggregateProtocol methods for its sample usage. You're going to modify TestAggregateProtocol in next patch, right ? public class DoubleColumnInterpreter extends ColumnInterpreter< Double , Double , EmptyMsg, DoubleMsg, DoubleMsg>{ Wrap long line.
        Hide
        Julian Wissmann added a comment -

        For starters here are the source files I wrote and modified - in case anyone wants to comment.
        I'll get a patch prepared from those some time in the next few days.

        Gegards
        Julian

        Show
        Julian Wissmann added a comment - For starters here are the source files I wrote and modified - in case anyone wants to comment. I'll get a patch prepared from those some time in the next few days. Gegards Julian
        Hide
        Julian Wissmann added a comment -

        I started working on implementing this on the trunk as chances are, that I might need this, myself at some point.

        Show
        Julian Wissmann added a comment - I started working on implementing this on the trunk as chances are, that I might need this, myself at some point.

          People

          • Assignee:
            Julian Wissmann
            Reporter:
            Ted Yu
          • Votes:
            1 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development