hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Sichi" <jsi...@fb.com>
Subject Re: Review Request: HIVE-1644 Use filter pushdown for automatically accessing indexes
Date Mon, 18 Apr 2011 19:24:55 GMT

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



conf/hive-default.xml
<https://reviews.apache.org/r/558/#comment1019>

    BTW, these property names should be all-lowercase.



ql/src/java/org/apache/hadoop/hive/ql/Driver.java
<https://reviews.apache.org/r/558/#comment1022>

    When you add an overload, add Javadoc as well (including the new param's meaning).
    



ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexQueryContext.java
<https://reviews.apache.org/r/558/#comment1020>

    Could you explain the usage interaction better (along the lines of what I explained in
my review comment)?
    



ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java
<https://reviews.apache.org/r/558/#comment1021>

    You're right.  Either we need to treat them as index columns (so that the predicates on
them will automatically be collected by the predicate analyzer), or we need to explicitly
generate corresponding equality predicates based on the partition values which have already
been identified by partition pruning.
    



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

    From an efficiency perspective, you certainly don't want to be doing this over and over
inside the outer for loop; just do it once first outside.
    
    Also, for a table with a huge number of partitions, fetching all of them is a bad idea;
it's better to selectively query the partitions of interest (but batching them if possible).
    



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

    This doesn't work because the Partition class does not override the default Java equals
method (which is based on object identity rather than value), and different metastore queries
return different object instances for the same underlying entity.



ql/src/test/queries/clientpositive/index_auto_multiple.q
<https://reviews.apache.org/r/558/#comment1025>

    I don't understand what you mean?  src has two columns, key and value.



ql/src/test/queries/clientpositive/index_auto_unused.q
<https://reviews.apache.org/r/558/#comment1026>

    From the index design doc, there's an optional PARTITION clause when rebuilding an index
which allows you to build just one specific partition, leaving the others unbuilt.  I think
there are some examples in the unit tests.
    
    ALTER INDEX index_name ON table_name [ PARTITION (...) ] REBUILD


- John


On 2011-04-16 06:04:26, Russell Melick wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/558/
> -----------------------------------------------------------
> 
> (Updated 2011-04-16 06:04:26)
> 
> 
> Review request for hive.
> 
> 
> Summary
> -------
> 
> Review request for HIVE-1644.12.patch
> 
> 
> This addresses bug HIVE-1644.
>     https://issues.apache.org/jira/browse/HIVE-1644
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java a21f589 
>   conf/hive-default.xml c42197f 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 14015d0 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java 6437385 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java c02d90b 
>   ql/src/java/org/apache/hadoop/hive/ql/index/AbstractIndexHandler.java dd0186d 
>   ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexHandler.java 411b78f 
>   ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexQueryContext.java PRE-CREATION

>   ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java 1f01446

>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 50db44c 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRTableScan1.java 6162676 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/IndexWhereResolver.java PRE-CREATION

>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/PhysicalOptimizer.java 0ae9fa2

>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcCtx.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereTaskDispatcher.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 937a7b3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java f0aca84 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/MapredWork.java 73391e9 
>   ql/src/test/queries/clientpositive/index_auto.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/index_auto_file_format.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/index_auto_multiple.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/index_auto_partitioned.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/index_auto_unused.q PRE-CREATION 
>   ql/src/test/results/clientpositive/index_auto.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/index_auto_file_format.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/index_auto_multiple.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/index_auto_partitioned.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/index_auto_unused.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/index_opt_where.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/index_opt_where_partitioned.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/index_opt_where_simple.q.out PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/558/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Russell
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message