----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/392/#review217 -----------------------------------------------------------
http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java <https://reviews.apache.org/r/392/#comment432> I suggest hive.optimize.index.groupby for this property. http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java <https://reviews.apache.org/r/392/#comment433> Typo: Unknown http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteCanApplyCtx.java <https://reviews.apache.org/r/392/#comment435> I don't understand why these state variables are maintained as conf variables rather than just data members of a class. Could you explain why that is necessary? http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteCanApplyCtx.java <https://reviews.apache.org/r/392/#comment434> Why is the exception being ignored here? http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteCanApplyProcFactory.java <https://reviews.apache.org/r/392/#comment437> Need Apache headers on all new files. http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteCanApplyProcFactory.java <https://reviews.apache.org/r/392/#comment436> Use "expr" instead of engfd http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteCanApplyProcFactory.java <https://reviews.apache.org/r/392/#comment440> Typo: groub-by http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteCanApplyProcFactory.java <https://reviews.apache.org/r/392/#comment506> What about nested function invocations? http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteCanApplyProcFactory.java <https://reviews.apache.org/r/392/#comment442> Eliminate commented-out code http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteCanApplyProcFactory.java <https://reviews.apache.org/r/392/#comment443> Need to abstract out this dependency on _c convention. http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteGBUsingIndex.java <https://reviews.apache.org/r/392/#comment507> For HIVE-1644, the HMC team is creating a new package org.apache.hadoop.hive.ql.optimizer.index. I think any optimizer code related to indexing should go in there. http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteGBUsingIndex.java <https://reviews.apache.org/r/392/#comment444> This code is currently tied to the compact index representation. We mentioned earlier that we'll need a new index representation (summary) instead in order to implement the counts correctly (we should leave the compact representation as is). So: * until the summary representation is added, we can't enable this * in general, it would be good to find a way to make this pluggable; for example, the bitmap index representation can also be utilized by counting the bits, but the rewrite expression would be slightly different http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteGBUsingIndex.java <https://reviews.apache.org/r/392/#comment445> TODO? http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteGBUsingIndex.java <https://reviews.apache.org/r/392/#comment446> Might want to use a finer level than LOG.info http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteGBUsingIndex.java <https://reviews.apache.org/r/392/#comment447> Just a sanity check to avoid huge payloads coming back from thrift. http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteGBUsingIndex.java <https://reviews.apache.org/r/392/#comment448> Hmm, no, I think we should fail hard here. If the underlying problem is fatal (e.g. the metastore went down), we should not try to hide it. - John On 2011-02-03 16:45:15, John Sichi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/392/ > ----------------------------------------------------------- > > (Updated 2011-02-03 16:45:15) > > > Review request for hive. > > > Summary > ------- > > Preliminary review. > > > This addresses bug HIVE-1694. > https://issues.apache.org/jira/browse/HIVE-1694 > > > Diffs > ----- > > http://svn.apache.org/repos/asf/hive/trunk/build.xml 1067048 > > http://svn.apache.org/repos/asf/hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > 1067048 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java > 1067048 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java > 1067048 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/Optimizer.java > 1067048 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteCanApplyCtx.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteCanApplyProcFactory.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteGBUsingIndex.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteIndexSubqueryCtx.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteIndexSubqueryProcFactory.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteParseContextGenerator.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteRemoveGroupbyCtx.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/optimizer/RewriteRemoveGroupbyProcFactory.java > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java > 1067048 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > 1067048 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/org/apache/hadoop/hive/ql/QTestUtil.java > 1067048 > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientnegative/fatal.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/queries/clientpositive/ql_rewrite_gbtoidx.q > PRE-CREATION > > http://svn.apache.org/repos/asf/hive/trunk/ql/src/test/results/clientpositive/ql_rewrite_gbtoidx.q.out > PRE-CREATION > > Diff: https://reviews.apache.org/r/392/diff > > > Testing > ------- > > > Thanks, > > John > >
