You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by John Sichi <js...@fb.com> on 2011/03/04 23:34:35 UTC

Review Request: HIVE-1803: Implement bitmap indexing in Hive

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/466/
-----------------------------------------------------------

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


Re: Review Request: HIVE-1803: Implement bitmap indexing in Hive

Posted by M IS <mi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/466/#review304
-----------------------------------------------------------



ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexTableIndexInputFormat.java
<https://reviews.apache.org/r/466/#comment597>

    It would be better if the declaration of the loop-private variables is done just once outside the loop and their values re-initialized in every iteration of the loop. Re-declaration in every iteration isn't such a good idea. This need be taken at many places i guess.



ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexTableIndexResult.java
<https://reviews.apache.org/r/466/#comment598>

    Please don't hard-code the class name. It becomes difficult in future if the code-refactoring is to be carried out. A better way would be as below:
    public static final Log l4j = LogFactory.getLog(HiveIndexTableIndexResult.class.getSimpleName());



ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexTableIndexResult.java
<https://reviews.apache.org/r/466/#comment599>

    If an exception is thrown, then the LineReader may not be closed at all, lead to a resource leakage. It would be a safe bet to protect the code in try {} block and close it in finally block.



ql/src/java/org/apache/hadoop/hive/ql/index/IndexMetadataChangeTask.java
<https://reviews.apache.org/r/466/#comment600>

    Using IndexMetadataChangeTask.class.getSimpleName() would be a better way, I suppose.



ql/src/java/org/apache/hadoop/hive/ql/index/IndexMetadataChangeWork.java
<https://reviews.apache.org/r/466/#comment601>

    A generated unique serialVersionUID is preferrable.


- M


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
> 
>