----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14856/#review27457 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java <https://reviews.apache.org/r/14856/#comment53352> Better name: postJoinFilters ? ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java <https://reviews.apache.org/r/14856/#comment53353> Instead of this lazy initialization, its just better to initialize eagerly and not do this null check. Makes code more succinct. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java <https://reviews.apache.org/r/14856/#comment53354> Better to move this function as a static function in ParseUtils to avoid further lengthening of SemanticAnalyzer. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java <https://reviews.apache.org/r/14856/#comment53364> Also, instead of QBJoinTree, this method just need to accept String[] aliases from it, that is joinTree.getBaseSrc(). No need to pass QBJoinTree in it. ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java <https://reviews.apache.org/r/14856/#comment53356> I am bit confused. Is it tablename.colname or colname ? Looks like former for test cases you added, so how come we are able to determine correct index for it in joinTree.baseSrc list, since that I presume will only contain table aliases? ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java <https://reviews.apache.org/r/14856/#comment53360> Do we also need to add following ? * HiveParser.TOK_NULL: * HiveParser.TOK_DATELITERAL: ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java <https://reviews.apache.org/r/14856/#comment53361> Per comment above, we could have this null check, if this list is always initialized with 0 size. There is nothing perf sensitive here, so code clarity is preferred. - Ashutosh Chauhan On Oct. 22, 2013, 8:52 p.m., Harish Butani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14856/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2013, 8:52 p.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Bugs: HIVE-5552 > https://issues.apache.org/jira/browse/HIVE-5552 > > > Repository: hive-git > > > Description > ------- > > Hive currently only support views in the FROM-clause, some Facebook use cases > suggest that Hive should support subqueries such as those connected by > IN/EXISTS in the WHERE-clause. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/parse/QBJoinTree.java 9834e73 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 063aa65 > ql/src/test/queries/clientpositive/join_merging.q PRE-CREATION > ql/src/test/results/clientpositive/join_merging.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/14856/diff/ > > > Testing > ------- > > ran all join tests. > added join_merging.q test > > > Thanks, > > Harish Butani > >
