----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1194/#review1303 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/index/AggregateIndexHandler.java <https://reviews.apache.org/r/1194/#comment2955> Can't you just look up "AGGREGATES" in the map? ql/src/java/org/apache/hadoop/hive/ql/index/AggregateIndexHandler.java <https://reviews.apache.org/r/1194/#comment2953> Add a helper method to avoid duplicating the code in the else block below. ql/src/java/org/apache/hadoop/hive/ql/index/AggregateIndexHandler.java <https://reviews.apache.org/r/1194/#comment2954> Can't you just look up "AGGREGATES" in the map? ql/src/java/org/apache/hadoop/hive/ql/index/AggregateIndexHandler.java <https://reviews.apache.org/r/1194/#comment2956> See recent changes in corresponding CompactIndexHandler code for HIVEOPTINDEXFILTER; need the same here (or better, factor out common code here and elsewhere). On a related note, you may be able to use the same technique instead of isQueryInsertToTable; this would be preferable since it's nice to be able to use the index rewrite in cases where it's a normal INSERT table with index being used for GROUP BY on SELECT from some other table. ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java <https://reviews.apache.org/r/1194/#comment2957> @params here don't match actual params ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteCanApplyProcFactory.java <https://reviews.apache.org/r/1194/#comment2958> Shouldn't this be the same as COUNT(*)? ql/src/test/queries/clientpositive/ql_rewrite_gbtoidx.q <https://reviews.apache.org/r/1194/#comment2980> Besides EXPLAIN, you should include a few queries against a non-empty table verifying that you get the correct results both with and without the optimization applied. Remember to include an ORDER BY for test determinism. ql/src/test/queries/clientpositive/ql_rewrite_gbtoidx.q <https://reviews.apache.org/r/1194/#comment2978> Isn't this set redundant? - John On 2011-08-03 10:31:42, Prajakta Kalmegh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1194/ > ----------------------------------------------------------- > > (Updated 2011-08-03 10:31:42) > > > Review request for hive and John Sichi. > > > Summary > ------- > > This patch has defined a new AggregateIndexHandler which is used to optimize > the query plan for groupby queries. > > > This addresses bug HIVE-1694. > https://issues.apache.org/jira/browse/HIVE-1694 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b46976f > ql/src/java/org/apache/hadoop/hive/ql/index/AggregateIndexHandler.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/index/HiveIndex.java 591c9ff > ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java a57f9cf > ql/src/java/org/apache/hadoop/hive/ql/optimizer/IndexUtils.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/optimizer/Optimizer.java 590d69a > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteCanApplyCtx.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteCanApplyProcFactory.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteGBUsingIndex.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteQueryUsingAggregateIndex.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteQueryUsingAggregateIndexCtx.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereProcessor.java > 8295687 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/physical/index/IndexWhereTaskDispatcher.java > 699519b > ql/src/test/queries/clientpositive/ql_rewrite_gbtoidx.q PRE-CREATION > ql/src/test/results/clientpositive/ql_rewrite_gbtoidx.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/1194/diff > > > Testing > ------- > > > Thanks, > > Prajakta > >
