[
https://issues.apache.org/jira/browse/HIVE-1644?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13017164#comment-13017164
]
jirapos...@reviews.apache.org commented on HIVE-1644:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/558/#review399
-----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/558/#comment748>
For consistency with my review in HIVE-1694, I suggest
hive.optimize.index.filter as the name for this configuration parameter.
(In HIVE-1694 I suggested hive.optimize.index.groupby, and we want it to be
possible to enable/disable them independently)
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/558/#comment749>
In line with the previous comment, suggest
hive.optimize.index.filter.compact.minSize/maxSize.
Namit's suggestion for minSize was 5G.
I think the default for maxSize should be infinity (I can't think of a case
where we want it in effect by default).
ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java
<https://reviews.apache.org/r/558/#comment750>
HIVE-1803 is changing this to hive.index.blockfilter.file. Assuming that
gets committed first, we should use that, since it's generic rather than tied
to the index type.
ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexHandler.java
<https://reviews.apache.org/r/558/#comment751>
What are the units here? Also, don't use colon after parameter name.
ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java
<https://reviews.apache.org/r/558/#comment752>
The non-functional changes in this file are gonna conflict with HIVE-1803,
so get rid of them.
ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java
<https://reviews.apache.org/r/558/#comment755>
Use HiveUtils.unparseIdentifier for quoting table names in generated SQL.
ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java
<https://reviews.apache.org/r/558/#comment756>
Isn't it incorrect to set properties on the original table scan here since
this is only tentative?
ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java
<https://reviews.apache.org/r/558/#comment757>
Likewise, modifying inputs is incorrect before we have a definite plan.
Some more work on the new HiveIndexHandler interface method is required for
resolving this plus the residuals.
ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java
<https://reviews.apache.org/r/558/#comment753>
If searchConditions.size() == 0, it means we didn't find anything which
could be handled by the index. In that case, we should bail out immediately
and not try to do anything more with this index.
ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java
<https://reviews.apache.org/r/558/#comment759>
We collect the residual here, but we don't do anything with it. Don't we
need to pass it back so that Hive can decide what to leave in the Filter
operator?
ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java
<https://reviews.apache.org/r/558/#comment760>
The list actually contains index objects, not index table names. Also
typo: "is exists"
ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRTableScan1.java
<https://reviews.apache.org/r/558/#comment761>
Only cast once.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
<https://reviews.apache.org/r/558/#comment764>
Indentation is wrong here.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
<https://reviews.apache.org/r/558/#comment763>
In my review for HIVE-1694, I noted that we should not be swallowing
exceptions. I think some of this code was copied from there. If we can't
access the metastore during optimization, it should be treated as a fatal error.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
<https://reviews.apache.org/r/558/#comment765>
The plan still looks wrong (there are two Stage-0's, one for the index
scan, one for the final fetch), so the relabeling is still not quite working
correctly.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
<https://reviews.apache.org/r/558/#comment766>
no space after !
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
<https://reviews.apache.org/r/558/#comment767>
Suggested rename for method: arePartitionsCoveredByIndex
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
<https://reviews.apache.org/r/558/#comment768>
This checks that the metadata matches. But it does not actually check that
the index partitions exist.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereTaskDispatcher.java
<https://reviews.apache.org/r/558/#comment769>
Is that true? Why couldn't index be used to optimize a map-only task?
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereTaskDispatcher.java
<https://reviews.apache.org/r/558/#comment770>
As noted above, we DO want to fail here.
ql/src/test/queries/clientpositive/index_opt_where.q
<https://reviews.apache.org/r/558/#comment772>
Could you add more tests for cases where automatic indexing should decide
not to kick in:
* index can't be used because of min/max size config
* index can't be used because predicate isn't supported
* index can't be used columns aren't covered
Also:
* case where multiple indexes apply and we pick one (currently arbitrarily,
but make sure it's at least deterministic so that test doesn't become flaky)
ql/src/test/queries/clientpositive/index_opt_where_partitioned.q
<https://reviews.apache.org/r/558/#comment771>
Could you add a test which shows the index NOT being used in the case where
required partitions haven't been built yet?
ql/src/test/results/clientpositive/index_opt_where_partitioned.q.out
<https://reviews.apache.org/r/558/#comment773>
Here's the duplicate Stage-0 I referred to in the code.
- John
On 2011-04-07 05:27:22, Russell Melick wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/558/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-04-07 05:27:22)
bq.
bq.
bq. Review request for hive.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Review request for HIVE-1644.12.patch
bq.
bq.
bq. This addresses bug HIVE-1644.
bq. https://issues.apache.org/jira/browse/HIVE-1644
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. ql/src/test/results/clientpositive/index_opt_where_simple.q.out
PRE-CREATION
bq. ql/src/test/results/clientpositive/index_opt_where_partitioned.q.out
PRE-CREATION
bq. ql/src/test/results/clientpositive/index_opt_where.q.out PRE-CREATION
bq. ql/src/java/org/apache/hadoop/hive/ql/plan/MapredWork.java 73391e9
bq. ql/src/test/queries/clientpositive/index_opt_where.q PRE-CREATION
bq. ql/src/test/queries/clientpositive/index_opt_where_partitioned.q
PRE-CREATION
bq. ql/src/test/queries/clientpositive/index_opt_where_simple.q PRE-CREATION
bq. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
f0aca84
bq.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereTaskDispatcher.java
PRE-CREATION
bq. ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 937a7b3
bq. ql/src/java/org/apache/hadoop/hive/ql/exec/ExecDriver.java 6437385
bq. ql/src/java/org/apache/hadoop/hive/ql/exec/TableScanOperator.java
c02d90b
bq. ql/src/java/org/apache/hadoop/hive/ql/index/AbstractIndexHandler.java
dd0186d
bq. ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndexHandler.java
411b78f
bq.
ql/src/java/org/apache/hadoop/hive/ql/index/compact/CompactIndexHandler.java
1f01446
bq. ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 50db44c
bq. ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMRTableScan1.java
6162676
bq.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/IndexWhereResolver.java
PRE-CREATION
bq.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/PhysicalOptimizer.java
0ae9fa2
bq.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcCtx.java
PRE-CREATION
bq.
ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java
PRE-CREATION
bq. common/src/java/org/apache/hadoop/hive/conf/HiveConf.java a21f589
bq. conf/hive-default.xml c42197f
bq.
bq. Diff: https://reviews.apache.org/r/558/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Russell
bq.
bq.
> use filter pushdown for automatically accessing indexes
> -------------------------------------------------------
>
> Key: HIVE-1644
> URL: https://issues.apache.org/jira/browse/HIVE-1644
> Project: Hive
> Issue Type: Improvement
> Components: Indexing
> Affects Versions: 0.8.0
> Reporter: John Sichi
> Assignee: Russell Melick
> Attachments: HIVE-1644.1.patch, HIVE-1644.10.patch,
> HIVE-1644.11.patch, HIVE-1644.12.patch, HIVE-1644.2.patch, HIVE-1644.3.patch,
> HIVE-1644.4.patch, HIVE-1644.5.patch, HIVE-1644.6.patch, HIVE-1644.7.patch,
> HIVE-1644.8.patch, HIVE-1644.9.patch
>
>
> HIVE-1226 provides utilities for analyzing filters which have been pushed
> down to a table scan. The next step is to use these for selecting available
> indexes and generating access plans for those indexes.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
