From 1cb14465707b0909d6381326d02e207d311c7870 Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Sun, 25 Aug 2013 17:36:53 -0700 Subject: [PATCH] HBASE-9283 Struct trailing null handling --- .../org/apache/hadoop/hbase/types/OrderedBlob.java | 1 + .../apache/hadoop/hbase/types/OrderedBlobVar.java | 1 + .../apache/hadoop/hbase/types/OrderedFloat32.java | 1 + .../apache/hadoop/hbase/types/OrderedFloat64.java | 1 + .../apache/hadoop/hbase/types/OrderedInt32.java | 1 + .../apache/hadoop/hbase/types/OrderedInt64.java | 1 + .../apache/hadoop/hbase/types/OrderedNumeric.java | 1 + .../apache/hadoop/hbase/types/OrderedString.java | 1 + .../java/org/apache/hadoop/hbase/types/Struct.java | 10 ++-- .../apache/hadoop/hbase/types/StructIterator.java | 2 +- .../org/apache/hadoop/hbase/types/TestStruct.java | 9 ++- .../hbase/types/TestStructNullExtension.java | 68 ++++++++++++++++++++++ 12 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedBlob.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedBlob.java index ddacea2..3f10095 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedBlob.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedBlob.java @@ -51,6 +51,7 @@ public class OrderedBlob extends OrderedBytesBase { @Override public byte[] decode(PositionedByteRange src) { + if (src.getPosition() == src.getLength()) return null; return OrderedBytes.decodeBlobCopy(src); } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedBlobVar.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedBlobVar.java index 45f1ae4..801f269 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedBlobVar.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedBlobVar.java @@ -47,6 +47,7 @@ public class OrderedBlobVar extends OrderedBytesBase { @Override public byte[] decode(PositionedByteRange src) { + if (src.getPosition() == src.getLength()) return null; return OrderedBytes.decodeBlobVar(src); } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedFloat32.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedFloat32.java index f0a8097..2d6cccf 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedFloat32.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedFloat32.java @@ -48,6 +48,7 @@ public class OrderedFloat32 extends OrderedBytesBase { @Override public Float decode(PositionedByteRange src) { + if (src.getPosition() == src.getLength()) return null; return OrderedBytes.decodeFloat32(src); } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedFloat64.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedFloat64.java index f60dbce..e92768d 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedFloat64.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedFloat64.java @@ -48,6 +48,7 @@ public class OrderedFloat64 extends OrderedBytesBase { @Override public Double decode(PositionedByteRange src) { + if (src.getPosition() == src.getLength()) return null; return OrderedBytes.decodeFloat64(src); } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedInt32.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedInt32.java index 95bfd8e..1382872 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedInt32.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedInt32.java @@ -48,6 +48,7 @@ public class OrderedInt32 extends OrderedBytesBase { @Override public Integer decode(PositionedByteRange src) { + if (src.getPosition() == src.getLength()) return null; return OrderedBytes.decodeInt32(src); } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedInt64.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedInt64.java index 54e5ad8..7ac462c 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedInt64.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedInt64.java @@ -48,6 +48,7 @@ public class OrderedInt64 extends OrderedBytesBase { @Override public Long decode(PositionedByteRange src) { + if (src.getPosition() == src.getLength()) return null; return OrderedBytes.decodeInt64(src); } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedNumeric.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedNumeric.java index 42636f1..41127dc 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedNumeric.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedNumeric.java @@ -56,6 +56,7 @@ public class OrderedNumeric extends OrderedBytesBase { @Override public Number decode(PositionedByteRange src) { + if (src.getPosition() == src.getLength()) return null; if (OrderedBytes.isNumericInfinite(src) || OrderedBytes.isNumericNaN(src)) { return OrderedBytes.decodeNumericAsDouble(src); } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedString.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedString.java index be464e8..951fd53 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedString.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/OrderedString.java @@ -47,6 +47,7 @@ public class OrderedString extends OrderedBytesBase { @Override public String decode(PositionedByteRange src) { + if (src.getPosition() == src.getLength()) return null; return OrderedBytes.decodeString(src); } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java index 8c2c38d..fa63e21 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/Struct.java @@ -104,9 +104,9 @@ public class Struct implements DataType { @SuppressWarnings("unchecked") @Override public int encodedLength(Object[] val) { - assert fields.length == val.length; + assert fields.length >= val.length; int sum = 0; - for (int i = 0; i < fields.length; i++) + for (int i = 0; i < val.length; i++) sum += fields[i].encodedLength(val[i]); return sum; } @@ -136,7 +136,7 @@ public class Struct implements DataType { int i = 0; Object[] ret = new Object[fields.length]; Iterator it = iterator(src); - while (it.hasNext()) + while (it.hasNext() && src.getPosition() <= src.getLength()) ret[i++] = it.next(); return ret; } @@ -155,9 +155,9 @@ public class Struct implements DataType { @SuppressWarnings("unchecked") @Override public int encode(PositionedByteRange dst, Object[] val) { - assert fields.length == val.length; + assert fields.length >= val.length; int written = 0; - for (int i = 0; i < fields.length; i++) { + for (int i = 0; i < val.length; i++) { written += fields[i].encode(dst, val[i]); } return written; diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java index 12b4225..e2320c0 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/types/StructIterator.java @@ -69,7 +69,7 @@ public class StructIterator implements Iterator { @Override public boolean hasNext() { - return idx < types.length; + return idx < types.length && src.getPosition() <= src.getLength(); } @Override diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java index 1ef17cd..f679035 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStruct.java @@ -154,11 +154,10 @@ public class TestStruct { final transient String str; public Pojo2(Object... vals) { - byte[] empty = new byte[0]; - byteField1Asc = vals.length > 0 ? (byte[]) vals[0] : empty; - byteField2Dsc = vals.length > 1 ? (byte[]) vals[1] : empty; - stringFieldDsc = vals.length > 2 ? (String) vals[2] : ""; - byteField3Dsc = vals.length > 3 ? (byte[]) vals[3] : empty; + byteField1Asc = vals.length > 0 ? (byte[]) vals[0] : null; + byteField2Dsc = vals.length > 1 ? (byte[]) vals[1] : null; + stringFieldDsc = vals.length > 2 ? (String) vals[2] : null; + byteField3Dsc = vals.length > 3 ? (byte[]) vals[3] : null; str = new StringBuilder() .append("{ ") .append(Bytes.toStringBinary(byteField1Asc)).append(", ") diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java new file mode 100644 index 0000000..e3a927a --- /dev/null +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/types/TestStructNullExtension.java @@ -0,0 +1,68 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.types; + +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; + +import org.apache.hadoop.hbase.SmallTests; +import org.apache.hadoop.hbase.util.PositionedByteRange; +import org.apache.hadoop.hbase.util.SimplePositionedByteRange; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category(SmallTests.class) +public class TestStructNullExtension { + + @Test + public void testNullExtension() { + // the following field members are used because they're all nullable + Struct shortest = new StructBuilder() + .add(OrderedInt32.ASCENDING) + .add(OrderedString.ASCENDING) + .toStruct(); + Struct extended = new StructBuilder() + .add(OrderedInt32.ASCENDING) + .add(OrderedString.ASCENDING) + // intentionally include a wrapped instance to test wrapper behavior. + .add(new TerminatedWrapper(OrderedString.ASCENDING, "/")) + .add(OrderedNumeric.ASCENDING) + .toStruct(); + PositionedByteRange buf1 = new SimplePositionedByteRange(10); + PositionedByteRange buf2 = new SimplePositionedByteRange(10); + + Object[] val1 = new Object[] { 1, "foo" }; + int expectedLen = shortest.encode(buf1, val1); + buf1.setPosition(0); + assertArrayEquals("Simple struct decoding is broken.", val1, shortest.decode(buf1)); + + buf1.setPosition(0); + assertArrayEquals("Decoding short value with extended struct should append null elements.", + Arrays.copyOf(val1, 4), extended.decode(buf1)); + + buf1.setPosition(0); + assertEquals( + "Encoding a short value with extended struct should have same result as using short struct.", + expectedLen, extended.encode(buf2, val1)); + assertArrayEquals( + "Encoding a short value with extended struct should have same result as using short struct", + buf1.getBytes(), buf2.getBytes()); + } +} -- 1.8.3.4