> On None, John Sichi wrote: > > ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapIndexHandler.java, > > line 50 > > <https://reviews.apache.org/r/466/diff/1/?file=13567#file13567line50> > > > > The point of factoring out the compact index handler base class was so > > that you could eliminate most of the code in this class, right? :)
I don't know what happened with the refactoring here. I swear I did it right. Anyways, I'll fix it in the next patch. > On None, John Sichi wrote: > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFCollectBitmapSet.java, > > line 46 > > <https://reviews.apache.org/r/466/diff/1/?file=13579#file13579line46> > > > > Couldn't you make this a single-parameter UDF which just tests whether > > a bitmap is empty or not? Then use the existing UDAF collect_set to > > collect the distinct block offsets. Duh. Should have thought of that. > On None, John Sichi wrote: > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFCollectBitmapSet.java, > > line 128 > > <https://reviews.apache.org/r/466/diff/1/?file=13579#file13579line128> > > > > Hmmm...looking at the EWAH code, we could actually make our decision by > > reading just the header to avoid having to deserialize the whole thing. I don't think we can. The header shows the actual size in bits, but all of those bits can be zero. I think we still need to deserialize the entire bitmap in order to decide. > On None, John Sichi wrote: > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFBitmap.java, > > line 193 > > <https://reviews.apache.org/r/466/diff/1/?file=13578#file13578line193> > > > > Couldn't you avoid this copying by having the BitmapObjectInput/Output > > already work in terms of LongWritable? When we convert the BitmapObjectInput/Output to work with LongWritables, we then need to worry about the bitmap_and, bitmap_or, (and the new bitmap_empty) udfs, since they take the hive datatype array<bigint> as an argument, which is never represented as an Array<LongWritable>. For the next patch I'm preparing, I think I'm just going to keep the copying code in those UDFs except copy stuff into an Array<LongWritable> to pass to the BitmapObjectInput constructor. Do you think it would be better to pass the entire array object to BitmapObjectInput along with the ListObjectInspector and other classes required to read from the array? - Marquis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/466/#review303 ----------------------------------------------------------- On 2011-03-04 14:34:35, John Sichi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/466/ > ----------------------------------------------------------- > > (Updated 2011-03-04 14:34:35) > > > Review request for hive. > > > Summary > ------- > > Review by JVS. > > > This addresses bug HIVE-1803. > https://issues.apache.org/jira/browse/HIVE-1803 > > > Diffs > ----- > > lib/README 1c2f0b1 > lib/javaewah.jar PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java af2bacb > ql/src/java/org/apache/hadoop/hive/ql/exec/MapOperator.java ff74f08 > > ql/src/java/org/apache/hadoop/hive/ql/index/AbstractIndexTableIndexHandler.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndex.java 308d985 > > ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexTableIndexInputFormat.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexTableIndexResult.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/index/IndexMetadataChangeTask.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/index/IndexMetadataChangeWork.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapIndexHandler.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapObjectInput.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapObjectOutput.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java > 1f01446 > > ql/src/java/org/apache/hadoop/hive/ql/index/compact/HiveCompactIndexInputFormat.java > 6c320c5 > > ql/src/java/org/apache/hadoop/hive/ql/index/compact/HiveCompactIndexResult.java > 0c9ccea > > ql/src/java/org/apache/hadoop/hive/ql/index/compact/IndexMetadataChangeTask.java > eac168f > > ql/src/java/org/apache/hadoop/hive/ql/index/compact/IndexMetadataChangeWork.java > 26beb4e > ql/src/java/org/apache/hadoop/hive/ql/io/HiveContextAwareRecordReader.java > 391e5de > ql/src/java/org/apache/hadoop/hive/ql/io/IOContext.java 77220a1 > ql/src/java/org/apache/hadoop/hive/ql/metadata/VirtualColumn.java 30714b8 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFBitmap.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDAFCollectBitmapSet.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBitmapAnd.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFBitmapOr.java > PRE-CREATION > ql/src/test/queries/clientpositive/index_bitmap.q PRE-CREATION > ql/src/test/queries/clientpositive/index_bitmap1.q PRE-CREATION > ql/src/test/queries/clientpositive/index_bitmap2.q PRE-CREATION > ql/src/test/queries/clientpositive/index_compact.q 6547a52 > ql/src/test/queries/clientpositive/index_compact_1.q 6d59353 > ql/src/test/queries/clientpositive/index_compact_2.q 358b5e9 > ql/src/test/queries/clientpositive/index_compact_3.q ee8abda > ql/src/test/queries/clientpositive/udf_bitmap_and.q PRE-CREATION > ql/src/test/queries/clientpositive/udf_bitmap_or.q PRE-CREATION > ql/src/test/results/clientpositive/index_bitmap.q.out PRE-CREATION > ql/src/test/results/clientpositive/index_bitmap1.q.out PRE-CREATION > ql/src/test/results/clientpositive/index_bitmap2.q.out PRE-CREATION > ql/src/test/results/clientpositive/udf_bitmap_and.q.out PRE-CREATION > ql/src/test/results/clientpositive/udf_bitmap_or.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/466/diff > > > Testing > ------- > > > Thanks, > > John > >
