----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/162/#review63 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java <https://reviews.apache.org/r/162/#comment46> Please change the type to Map. ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java <https://reviews.apache.org/r/162/#comment47> Ditto ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java <https://reviews.apache.org/r/162/#comment49> Please change the return type to Map. ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java <https://reviews.apache.org/r/162/#comment50> Ditto. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java <https://reviews.apache.org/r/162/#comment51> It looks like there is a fair amount of duplicated code between this method and genFilterPlan(String dest, QB qb, Operator input). If possible can you change the name of genFilterPlan(String, QB, Operator) to getWherePlan(String, QB, Operator), and have both this method and genHavingPlan() call a common genFilterPlan(QB, ASTNode, Operator) method? ql/src/test/queries/clientpositive/having.q <https://reviews.apache.org/r/162/#comment64> Please add the following test queries: * Queries that contain both WHERE and HAVING clauses. * A query with an aggregate function in the HAVING clause that also appears in the SELECT clause without an alias. * A query that contains a HAVING clause but no GROUP BY clause. - Carl On 2010-12-09 15:59:46, Carl Steinbach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/162/ > ----------------------------------------------------------- > > (Updated 2010-12-09 15:59:46) > > > Review request for hive. > > > Summary > ------- > > Review of HIVE-1790 by CWS. > > > This addresses bug HIVE-1790. > https://issues.apache.org/jira/browse/HIVE-1790 > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/parse/Hive.g 6b47702 > ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java c36adc7 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java c667a63 > ql/src/test/queries/clientpositive/having.q PRE-CREATION > ql/src/test/results/clientpositive/having.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/162/diff > > > Testing > ------- > > > Thanks, > > Carl > >
