HBase
  1. HBase
  2. HBASE-9283

Struct and StructIterator should properly handle trailing nulls

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.95.2
    • Fix Version/s: 0.98.0, 0.96.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Tags:
      Phoenix

      Description

      For a composite row key, Phoenix strips off trailing null columns values in the row key. The reason this is important is that then new nullable row key columns can be added to a schema without requiring any data upgrade to existing rows. Otherwise, adding new row key columns to the end of a schema becomes extremely cumbersome, as you'd need to delete all existing rows and add them back with a row key that includes a null value.

      Rather than Phoenix needing to modify the iteration code everywhere (as Nick Dimiduk outlined here: https://issues.apache.org/jira/browse/HBASE-8693?focusedCommentId=13744499&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13744499), it'd be better if StructIterator handled this out-of-the-box. Otherwise, if Phoenix has to specialize this, we'd lose the interop piece which is the justification for switching our type system to this new one in the first place.

        Issue Links

          Activity

          Hide
          Nick Dimiduk added a comment -

          Hi James Taylor.

          Have a look at this patch. TestStructNullExtension is designed to demonstrate the behavior we discussed. Notice I had to change all the nullable types. The difficulty is in determining whether the last entry is an implied null or something like an empty RawByte instance. The solution I have here is to require that any nullable type check for src.getPosition() == src.getLength() and return null instead of error out. Without this change, TestStruct fails precisely because of this discrepancy.

          Show
          Nick Dimiduk added a comment - Hi James Taylor . Have a look at this patch. TestStructNullExtension is designed to demonstrate the behavior we discussed. Notice I had to change all the nullable types. The difficulty is in determining whether the last entry is an implied null or something like an empty RawByte instance. The solution I have here is to require that any nullable type check for src.getPosition() == src.getLength() and return null instead of error out. Without this change, TestStruct fails precisely because of this discrepancy.
          Hide
          Nick Dimiduk added a comment -

          I realized I don't need to push this position checking down into the DataType implementations. This patch keeps the null extension logic localized to Struct and StructIterator. I'll post one more version tomorrow which updates the docs.

          Show
          Nick Dimiduk added a comment - I realized I don't need to push this position checking down into the DataType implementations. This patch keeps the null extension logic localized to Struct and StructIterator. I'll post one more version tomorrow which updates the docs.
          Hide
          James Taylor added a comment -

          +1, I like your new impl that keeps the null extension logic localized to Struct and StructIterator.

          Show
          James Taylor added a comment - +1, I like your new impl that keeps the null extension logic localized to Struct and StructIterator.
          Hide
          Nick Dimiduk added a comment -

          Updated patch. More rigorous unit tests and updated documentation. Depends on HBASE-9348.

          Show
          Nick Dimiduk added a comment - Updated patch. More rigorous unit tests and updated documentation. Depends on HBASE-9348 .
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12600084/0001-HBASE-9283-Struct-trailing-null-handling.patch
          against trunk revision .

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

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

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

          +1 hadoop2.0. The patch compiles against the hadoop 2.0 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 does not introduce lines longer than 100

          +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/6935//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//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/12600084/0001-HBASE-9283-Struct-trailing-null-handling.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 8 new or modified tests. +1 hadoop1.0 . The patch compiles against the hadoop 1.0 profile. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 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 does not introduce lines longer than 100 +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/6935//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/6935//console This message is automatically generated.
          Hide
          stack added a comment -

          Committed to 0.95 and trunk. Thanks for the patch Nick.

          Show
          stack added a comment - Committed to 0.95 and trunk. Thanks for the patch Nick.
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in hbase-0.95 #500 (See https://builds.apache.org/job/hbase-0.95/500/)
          HBASE-9283 Struct and StructIterator should properly handle trailing nulls (stack: rev 1518319)

          • /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java
          • /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java
          • /hbase/branches/0.95/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java
          • /hbase/branches/0.95/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java
          Show
          Hudson added a comment - SUCCESS: Integrated in hbase-0.95 #500 (See https://builds.apache.org/job/hbase-0.95/500/ ) HBASE-9283 Struct and StructIterator should properly handle trailing nulls (stack: rev 1518319) /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java /hbase/branches/0.95/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java /hbase/branches/0.95/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in HBase-TRUNK #4441 (See https://builds.apache.org/job/HBase-TRUNK/4441/)
          HBASE-9283 Struct and StructIterator should properly handle trailing nulls (stack: rev 1518320)

          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java
          • /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java
          • /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java
          Show
          Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #4441 (See https://builds.apache.org/job/HBase-TRUNK/4441/ ) HBASE-9283 Struct and StructIterator should properly handle trailing nulls (stack: rev 1518320) /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in hbase-0.95-on-hadoop2 #276 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/276/)
          HBASE-9283 Struct and StructIterator should properly handle trailing nulls (stack: rev 1518319)

          • /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java
          • /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java
          • /hbase/branches/0.95/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java
          • /hbase/branches/0.95/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java
          Show
          Hudson added a comment - SUCCESS: Integrated in hbase-0.95-on-hadoop2 #276 (See https://builds.apache.org/job/hbase-0.95-on-hadoop2/276/ ) HBASE-9283 Struct and StructIterator should properly handle trailing nulls (stack: rev 1518319) /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java /hbase/branches/0.95/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java /hbase/branches/0.95/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java /hbase/branches/0.95/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java
          Hide
          Hudson added a comment -

          FAILURE: Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #700 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/700/)
          HBASE-9283 Struct and StructIterator should properly handle trailing nulls (stack: rev 1518320)

          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java
          • /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java
          • /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java
          • /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java
          Show
          Hudson added a comment - FAILURE: Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #700 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/700/ ) HBASE-9283 Struct and StructIterator should properly handle trailing nulls (stack: rev 1518320) /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java /hbase/trunk/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java /hbase/trunk/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java

            People

            • Assignee:
              Nick Dimiduk
              Reporter:
              Nick Dimiduk
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development