----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20737/#review41607 -----------------------------------------------------------
Few comments. Major one is to drop subquery from explain subquery rewrite syntax. ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java <https://reviews.apache.org/r/20737/#comment75143> will be good to add a comment here for this replacement. ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java <https://reviews.apache.org/r/20737/#comment75133> Is this needed ? Looks redundant code. If not, good to put some comments here. ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g <https://reviews.apache.org/r/20737/#comment75132> I think we should not have explain subquery rewrite .. rather explain rewrite .. because later on we may want to extend this explain functionality beyond just subquery rewrites. ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java <https://reviews.apache.org/r/20737/#comment75146> Doesn't make lot of difference, but QBSQRewrite extends QBSQRewriteNoop seems more natural. ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java <https://reviews.apache.org/r/20737/#comment75147> Also, good to add a note here saying its non-explain path. - Ashutosh Chauhan On April 25, 2014, 10:57 p.m., Harish Butani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20737/ > ----------------------------------------------------------- > > (Updated April 25, 2014, 10:57 p.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Bugs: HIVE-6031 > https://issues.apache.org/jira/browse/HIVE-6031 > > > Repository: hive-git > > > Description > ------- > > explain subquery rewrite for where clause predicates > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java 679c6ec > > ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 3e673ca > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13bbf0a > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 864e692 > ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java a8b436e > ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java b7c9e65 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 3b33dc2 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java > e7d0359 > ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryUtils.java 07d32ed > ql/src/java/org/apache/hadoop/hive/ql/plan/ExplainSQRewriteWork.java > PRE-CREATION > ql/src/test/queries/clientpositive/subquery_exists_explain_rewrite.q > PRE-CREATION > ql/src/test/queries/clientpositive/subquery_in_explain_rewrite.q > PRE-CREATION > ql/src/test/results/clientpositive/subquery_exists_explain_rewrite.q.out > PRE-CREATION > ql/src/test/results/clientpositive/subquery_in_explain_rewrite.q.out > PRE-CREATION > > Diff: https://reviews.apache.org/r/20737/diff/ > > > Testing > ------- > > new tests added > > > Thanks, > > Harish Butani > >
