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



ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexHandler.java
<https://reviews.apache.org/r/857/#comment1666>

    Update Javadoc and param name, including an explanation of what handler is 
supposed to do when multiple indexes are passed in.



ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapIndexHandler.java
<https://reviews.apache.org/r/857/#comment1675>

    I'm confused by the logic here.  You are throwing together all of the 
columns for all of the indexes, but we need to keep them segregated, don't we?  
Each subquery should only contain references to the columns relevant to the 
corresponding index.
    
    (But the partitioning predicates need to be applied to each index.)
    



ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapInnerQuery.java
<https://reviews.apache.org/r/857/#comment1668>

    Why is this public instead of private?



ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapInnerQuery.java
<https://reviews.apache.org/r/857/#comment1667>

    Use HiveUtils.unparseIdentifier



ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/HiveBitmapIndexInputFormat.java
<https://reviews.apache.org/r/857/#comment1669>

    Why do we need this class at all?  The superclass already uses 
hive.index.blockfilter.file by default.
    



ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
<https://reviews.apache.org/r/857/#comment1672>

    Seems like we should only be looking at the indexes on the table accessed 
by this table scan.  (This comment is retroactive to the original version of 
the file.)
    



ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
<https://reviews.apache.org/r/857/#comment1673>

    Seems like the costing comment below applies to this too.



ql/src/test/queries/clientpositive/index_bitmap3.q
<https://reviews.apache.org/r/857/#comment1670>

    Why do we need this setting at all?  (I'm not sure why it was there in the 
original version of the file.)


- John


On 2011-06-06 21:37:38, Syed Albiz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/857/
> -----------------------------------------------------------
> 
> (Updated 2011-06-06 21:37:38)
> 
> 
> Review request for hive and John Sichi.
> 
> 
> Summary
> -------
> 
> Add support for generating index queries to support automatic usage of bitmap 
> indexes. This required changing the interface to the IndexHandlers to support 
> accepting queries on multiple indexes. The compact indexes were modified to 
> use this new interface as well, although no functional changes were made to 
> how they work. Only supports AND predicates right now, but it should be 
> possibly to extend the BitmapQuery interface defined in this patch to easily 
> support OR predicates as well. Currently benchmarking these changes on a test 
> cluster.
> 
> 
> This addresses bug HIVE-2036.
>     https://issues.apache.org/jira/browse/HIVE-2036
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java 4fba845 
>   ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexHandler.java e5ee183 
>   ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapIndexHandler.java 
> af9d7b1 
>   ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapInnerQuery.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapOuterQuery.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/BitmapQuery.java 
> PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/index/bitmap/HiveBitmapIndexInputFormat.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java 
> 56e7609 
>   ql/src/java/org/apache/hadoop/hive/ql/io/HiveFileFormatUtils.java d64e88b 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
>  268560d 
>   
> ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereTaskDispatcher.java
>  0873e1a 
>   ql/src/test/queries/clientpositive/index_bitmap3.q 508eb94 
>   ql/src/test/queries/clientpositive/index_bitmap_auto.q PRE-CREATION 
>   ql/src/test/results/clientpositive/index_bitmap_auto.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/857/diff
> 
> 
> Testing
> -------
> 
> Passes unit tests, additional testcase to test automatic bitmap indexing 
> index_bitmap_auto.q was also added to the TestCliDriver suite. Currently 
> benchmarking changes on a test cluster.
> 
> 
> Thanks,
> 
> Syed
> 
>

Reply via email to