You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pig.apache.org by th...@apache.org on 2010/11/17 22:24:03 UTC
svn commit: r1036234 - in /pig/branches/branch-0.8: CHANGES.txt
src/org/apache/pig/data/BinInterSedes.java
test/org/apache/pig/test/TestPigTupleRawComparator.java
Author: thejas
Date: Wed Nov 17 21:24:03 2010
New Revision: 1036234
URL: http://svn.apache.org/viewvc?rev=1036234&view=rev
Log:
PIG-1720: java.lang.NegativeArraySizeException during Quicksort
Modified:
pig/branches/branch-0.8/CHANGES.txt
pig/branches/branch-0.8/src/org/apache/pig/data/BinInterSedes.java
pig/branches/branch-0.8/test/org/apache/pig/test/TestPigTupleRawComparator.java
Modified: pig/branches/branch-0.8/CHANGES.txt
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.8/CHANGES.txt?rev=1036234&r1=1036233&r2=1036234&view=diff
==============================================================================
--- pig/branches/branch-0.8/CHANGES.txt (original)
+++ pig/branches/branch-0.8/CHANGES.txt Wed Nov 17 21:24:03 2010
@@ -207,6 +207,8 @@ PIG-1309: Map-side Cogroup (ashutoshc)
BUG FIXES
+PIG-1720: java.lang.NegativeArraySizeException during Quicksort (thejas)
+
PIG-1715: pig-withouthadoop.jar missing automaton.jar (thejas)
PIG-1727: Hadoop default config override pig.properties (rding)
Modified: pig/branches/branch-0.8/src/org/apache/pig/data/BinInterSedes.java
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.8/src/org/apache/pig/data/BinInterSedes.java?rev=1036234&r1=1036233&r2=1036234&view=diff
==============================================================================
--- pig/branches/branch-0.8/src/org/apache/pig/data/BinInterSedes.java (original)
+++ pig/branches/branch-0.8/src/org/apache/pig/data/BinInterSedes.java Wed Nov 17 21:24:03 2010
@@ -642,12 +642,8 @@ public class BinInterSedes implements In
int result = 0;
try {
// first compare sizes
- byte dt1 = bb1.get();
- byte dt2 = bb2.get();
- byte sizeType1 = getSizeType(dt1);
- byte sizeType2 = getSizeType(dt2);
- int tsz1 = readInt(bb1, sizeType1);
- int tsz2 = readInt(bb2, sizeType2);
+ int tsz1 = readSize(bb1);
+ int tsz2 = readSize(bb2);
if (tsz1 > tsz2)
return 1;
else if (tsz1 < tsz2)
@@ -775,10 +771,8 @@ public class BinInterSedes implements In
type1 = DataType.BYTEARRAY;
type2 = getGeneralizedDataType(dt2);
if (type1 == type2) {
- byte sizeType1 = getSizeType(dt1);
- byte sizeType2 = getSizeType(dt2);
- int basz1 = readInt(bb1, sizeType1);
- int basz2 = readInt(bb2, sizeType2);
+ int basz1 = readSize(bb1, dt1);
+ int basz2 = readSize(bb2, dt2);
byte[] ba1 = new byte[basz1];
byte[] ba2 = new byte[basz2];
bb1.get(ba1);
@@ -792,10 +786,8 @@ public class BinInterSedes implements In
type1 = DataType.CHARARRAY;
type2 = getGeneralizedDataType(dt2);
if (type1 == type2) {
- byte sizeType1 = getSizeType(dt1);
- byte sizeType2 = getSizeType(dt2);
- int casz1 = readInt(bb1, sizeType1);
- int casz2 = readInt(bb2, sizeType2);
+ int casz1 = readSize(bb1, dt1);
+ int casz2 = readSize(bb2, dt2);
byte[] ca1 = new byte[casz1];
byte[] ca2 = new byte[casz2];
bb1.get(ca1);
@@ -820,10 +812,8 @@ public class BinInterSedes implements In
type2 = getGeneralizedDataType(dt2);
if (type1 == type2) {
// first compare sizes
- byte sizeType1 = getSizeType(dt1);
- byte sizeType2 = getSizeType(dt2);
- int tsz1 = readInt(bb1, sizeType1);
- int tsz2 = readInt(bb2, sizeType2);
+ int tsz1 = readSize(bb1, dt1);
+ int tsz2 = readSize(bb2, dt2);
if (tsz1 > tsz2)
return 1;
else if (tsz1 < tsz2)
@@ -976,10 +966,8 @@ public class BinInterSedes implements In
int l1 = bb1.remaining();
int l2 = bb2.remaining();
// first compare sizes
- byte sizeType1 = getSizeType(dt1);
- byte sizeType2 = getSizeType(dt2);
- int bsz1 = readInt(bb1, sizeType1);
- int bsz2 = readInt(bb2, sizeType2);
+ int bsz1 = readSize(bb1, dt1);
+ int bsz2 = readSize(bb2, dt2);
if (bsz1 > bsz2)
return 1;
else if (bsz1 < bsz2)
@@ -1005,10 +993,8 @@ public class BinInterSedes implements In
int l1 = bb1.remaining();
int l2 = bb2.remaining();
// first compare sizes
- byte sizeType1 = getSizeType(dt1);
- byte sizeType2 = getSizeType(dt2);
- int bsz1 = readInt(bb1, sizeType1);
- int bsz2 = readInt(bb2, sizeType2);
+ int bsz1 = readSize(bb1, dt1);
+ int bsz2 = readSize(bb2, dt2);
if (bsz1 > bsz2)
return 1;
else if (bsz1 < bsz2)
@@ -1091,30 +1077,54 @@ public class BinInterSedes implements In
throw new RuntimeException("Unexpected data type " + type + " found in stream.");
}
}
+
+ /**
+ * @param bb ByteBuffer having serialized object, including the type information
+ * @param type serialized type information
+ * @return the size of this type
+ */
+ private static int readSize(ByteBuffer bb) {
+ return readSize(bb, bb.get());
+ }
- private static byte getSizeType(byte type) {
+ /**
+ * @param bb ByteBuffer having serialized object, minus the type information
+ * @param type serialized type information
+ * @return the size of this type
+ */
+ private static int readSize(ByteBuffer bb, byte type) {
switch (type) {
case BinInterSedes.TINYBYTEARRAY:
case BinInterSedes.TINYTUPLE:
case BinInterSedes.TINYBAG:
case BinInterSedes.TINYMAP:
- return BinInterSedes.INTEGER_INBYTE;
+ return getUnsignedByte(bb);
case BinInterSedes.SMALLBYTEARRAY:
case BinInterSedes.SMALLCHARARRAY:
case BinInterSedes.SMALLTUPLE:
case BinInterSedes.SMALLBAG:
case BinInterSedes.SMALLMAP:
- return BinInterSedes.INTEGER_INSHORT;
+ return getUnsignedShort(bb);
case BinInterSedes.BYTEARRAY:
case BinInterSedes.CHARARRAY:
case BinInterSedes.TUPLE:
case BinInterSedes.BAG:
case BinInterSedes.MAP:
- return BinInterSedes.INTEGER;
+ return bb.getInt();
default:
throw new RuntimeException("Unexpected data type " + type + " found in stream.");
}
}
+
+ //same as format used by DataInput/DataOutput for unsigned short
+ private static int getUnsignedShort(ByteBuffer bb) {
+ return (((bb.get() & 0xff) << 8) | (bb.get() & 0xff));
+ }
+
+ //same as format used by DataInput/DataOutput for unsigned byte
+ private static int getUnsignedByte(ByteBuffer bb) {
+ return bb.get() & 0xff;
+ }
}
@Override
Modified: pig/branches/branch-0.8/test/org/apache/pig/test/TestPigTupleRawComparator.java
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.8/test/org/apache/pig/test/TestPigTupleRawComparator.java?rev=1036234&r1=1036233&r2=1036234&view=diff
==============================================================================
--- pig/branches/branch-0.8/test/org/apache/pig/test/TestPigTupleRawComparator.java (original)
+++ pig/branches/branch-0.8/test/org/apache/pig/test/TestPigTupleRawComparator.java Wed Nov 17 21:24:03 2010
@@ -29,11 +29,11 @@ import java.util.Collections;
import java.util.List;
import java.util.Random;
-import org.apache.hadoop.conf.Configurable;
import org.apache.hadoop.io.RawComparator;
import org.apache.hadoop.mapred.JobConf;
import org.apache.pig.backend.hadoop.executionengine.mapReduceLayer.PigTupleDefaultRawComparator;
import org.apache.pig.backend.hadoop.executionengine.mapReduceLayer.PigTupleSortComparator;
+import org.apache.pig.data.DataBag;
import org.apache.pig.data.DataByteArray;
import org.apache.pig.data.DefaultDataBag;
import org.apache.pig.data.Tuple;
@@ -130,24 +130,64 @@ public class TestPigTupleRawComparator {
NullableTuple t = new NullableTuple(tf.newTuple(list));
int res = compareHelper(prototype, t, comparator);
assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
+
list.set(6, new DataByteArray(new byte[] { 0x20 }));
t = new NullableTuple(tf.newTuple(list));
res = compareHelper(prototype, t, comparator);
assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
+
+ // bytearray that will fit into BinInterSedes.TINYBYTEARRAY
+ String largeTinyStr = appendChars("abc", 'x', 255 - 10);
+ list.set(6, new DataByteArray(largeTinyStr));
+ t = new NullableTuple(tf.newTuple(list));
+ res = compareHelper(prototype, t, comparator);
+ assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
+
+ //longest bytearray that will fit into BinInterSedes.TINYBYTEARRAY
+ largeTinyStr = appendChars("", 'x', 255);
+ list.set(6, new DataByteArray(largeTinyStr));
+ t = new NullableTuple(tf.newTuple(list));
+ res = compareHelper(prototype, t, comparator);
+ assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
+
+ // bytearray that will fit into BinInterSedes.SMALLBYTEARRAY
+ String largeSmallStr = appendChars("abc", 'x', 65535 - 100);
+ list.set(6, new DataByteArray(largeSmallStr));
+ t = new NullableTuple(tf.newTuple(list));
+ res = compareHelper(prototype, t, comparator);
+ assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
+
+ // bytearray that will fit into BinInterSedes.BYTEARRAY
+ String largeStr = appendChars("abc", 'x', 65535 + 10000);
+ list.set(6, new DataByteArray(largeStr));
+ t = new NullableTuple(tf.newTuple(list));
+ res = compareHelper(prototype, t, comparator);
+ assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
+
}
- @Test
+ @Test
public void testCompareCharArray() throws IOException {
list.set(7, "hello world!");
NullableTuple t = new NullableTuple(tf.newTuple(list));
int res = compareHelper(prototype, t, comparator);
assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
assertTrue(res == 0);
+
list.set(7, "hello worlc!");
t = new NullableTuple(tf.newTuple(list));
res = compareHelper(prototype, t, comparator);
assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
assertTrue(res > 0);
+
+ // chararray that will fit into BinInterSedes.SMALLCHARARRAY
+ String largeTinyString = appendChars("hello worlc!", 'x', 300);
+ list.set(7, largeTinyString);
+ t = new NullableTuple(tf.newTuple(list));
+ res = compareHelper(prototype, t, comparator);
+ assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
+ assertTrue(res > 0);
+
list.set(7, "hello worlz!");
t = new NullableTuple(tf.newTuple(list));
res = compareHelper(prototype, t, comparator);
@@ -165,33 +205,61 @@ public class TestPigTupleRawComparator {
assertTrue(res < 0);
}
+ private String appendChars(String str, char c, int rep) {
+ StringBuilder sb = new StringBuilder(str.length() + rep);
+ sb.append(str);
+ for(int i=0; i<rep; i++){
+ sb.append(c);
+ }
+ return sb.toString();
+ }
+
@Test
public void compareInnerTuples() throws IOException {
NullableTuple t = new NullableTuple(tf.newTuple(list));
int res = compareHelper(prototype, t, comparator);
assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
assertTrue(res == 0);
+
list.set(8, tf.newTuple(Arrays.<Object> asList(8.0, 9f, 10l, 12)));
t = new NullableTuple(tf.newTuple(list));
res = compareHelper(prototype, t, comparator);
assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
assertTrue(res < 0);
+
list.set(8, tf.newTuple(Arrays.<Object> asList(8.0, 9f, 9l, 12)));
t = new NullableTuple(tf.newTuple(list));
res = compareHelper(prototype, t, comparator);
assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
assertTrue(res > 0);
+
list.set(8, tf.newTuple(Arrays.<Object> asList(7.0, 9f, 9l, 12)));
t = new NullableTuple(tf.newTuple(list));
res = compareHelper(prototype, t, comparator);
assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
assertTrue(res > 0);
+
+ //Tuple that will fit into BinInterSedes.TINYTUPLE
+ Tuple tinyTuple = createLargeTuple(1, 200, tf);
+ list.set(8, tinyTuple);
+ t = new NullableTuple(tf.newTuple(list));
+ res = compareHelper(prototype, t, comparator);
+ assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
+
+ //Tuple that will fit into BinInterSedes.SMALLTUPLE
+ Tuple smallTuple = createLargeTuple(1, 1000, tf);
+ list.set(8, smallTuple);
+ t = new NullableTuple(tf.newTuple(list));
+ res = compareHelper(prototype, t, comparator);
+ assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
+
// DataType.LONG < DataType.DOUBLE
list.set(8, tf.newTuple(Arrays.<Object> asList(8l, 9f, 9l, 12)));
t = new NullableTuple(tf.newTuple(list));
res = compareHelper(prototype, t, comparator);
assertEquals(Math.signum(prototype.compareTo(t)), Math.signum(res), 0);
assertTrue(res > 0);
+
// object after tuple
list = new ArrayList<Object>(list);
list.add(10);
@@ -201,6 +269,7 @@ public class TestPigTupleRawComparator {
res = compareHelper(t1, t2, comparator);
assertEquals(Math.signum(t1.compareTo(t2)), Math.signum(res), 0);
assertTrue(res < 0);
+
// fancy tuple nesting
list.set(list.size() - 1, tf.newTuple(list));
t1 = new NullableTuple(tf.newTuple(list));
@@ -212,6 +281,14 @@ public class TestPigTupleRawComparator {
assertTrue(res > 0);
}
+ private Tuple createLargeTuple(int num, int repetitions, TupleFactory tf) {
+ ArrayList<Integer> ar = new ArrayList<Integer>(repetitions);
+ for(int i=0; i<repetitions; i++){
+ ar.add(i, num);
+ }
+ return tf.newTuple(ar);
+ }
+
@Test
public void testCompareDataBag() throws IOException {
list = new ArrayList<Object>(list);
@@ -222,6 +299,28 @@ public class TestPigTupleRawComparator {
int res = compareHelper(t1, t2, comparator);
assertEquals(Math.signum(t1.compareTo(t2)), Math.signum(res), 0);
assertTrue(res < 0);
+
+ //bag that will fit into BinInterSedes.TINYBAG
+ DataBag largeBag = createLargeBag(200, tf);
+ t2 = new NullableTuple(tf.newTuple(largeBag));
+ res = compareHelper(t1, t2, comparator);
+ assertEquals(Math.signum(t1.compareTo(t2)), Math.signum(res), 0);
+
+ //bag that will fit into BinInterSedes.SMALLBAG
+ largeBag = createLargeBag(3000, tf);
+ t2 = new NullableTuple(tf.newTuple(largeBag));
+ res = compareHelper(t1, t2, comparator);
+ assertEquals(Math.signum(t1.compareTo(t2)), Math.signum(res), 0);
+
+ }
+
+ private DataBag createLargeBag(int size, TupleFactory tf) {
+ Tuple t = tf.newTuple(Arrays.asList(0));
+ ArrayList<Tuple> tuplist = new ArrayList<Tuple>(size);
+ for(int i=0; i<size; i++){
+ tuplist.add(t);
+ }
+ return new DefaultDataBag(tuplist);
}
@Test