> On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > > Lines 2829-2837 (original), 2829-2837 (patched) > > <https://reviews.apache.org/r/63343/diff/5/?file=1872020#file1872020line2829> > > > > This comments need to be updated. Way outdated.
I have updated this comment. > On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > > Line 2845 (original), 2845 (patched) > > <https://reviews.apache.org/r/63343/diff/5/?file=1872020#file1872020line2845> > > > > Seems like this doesn't throw anymore. Can remove SemanticException. It does, actually _parseJoinCondPopulateAlias_ throws it. > On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > > Lines 8278 (patched) > > <https://reviews.apache.org/r/63343/diff/5/?file=1872020#file1872020line8278> > > > > Whats the relation b/w this and insertSelectForSemijoin() ? Do we need > > both Yes, this is independent. The _insertSelectForSemijoin_ method will introduce a Project *below* the right input of the join to select only the columns from the right input that are needed for join processing, i.e., that are needed by the semijoin condition. In turn, this block introduces a Project *above* the semijoin operator to remove all the columns from the right hand side. This is only needed when there are residual conditions to evaluate by the semijoin, since it needs to see all the input columns to evaluate the condition (otherwise, the semijoin itself prunes the columns coming from that input). > On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > > Lines 8545 (patched) > > <https://reviews.apache.org/r/63343/diff/5/?file=1872020#file1872020line8547> > > > > Can't expr be of type different than ExprNodeColumnDesc or > > ExprNodeConstantDesc? If not, need to throw in those cases. Thanks, that was a good catch! As you mentioned, although we can only generate from column references, we might generate another type of expressions, e.g., field expressions for nested fields. Since this is an additional optimization, I have changed the code so we just return the input operator in case we find an expression that we cannot recognize, without introducing the Select+GroupBy chain. I added an additional test for nested fields. > On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > > Lines 8594 (patched) > > <https://reviews.apache.org/r/63343/diff/5/?file=1872020#file1872020line8607> > > > > other exprNode types? Same as above. > On Oct. 30, 2017, 3:19 p.m., Ashutosh Chauhan wrote: > > ql/src/test/queries/clientpositive/semijoin6.q > > Lines 18 (patched) > > <https://reviews.apache.org/r/63343/diff/5/?file=1872022#file1872022line18> > > > > If left semi join supports null-safe join, can you also add test for: > > select * from tx1 u left semi join tx2 v on u.b <=> v.b; I just added it. - Jesús ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63343/#review189588 ----------------------------------------------------------- On Oct. 28, 2017, 8:06 p.m., Jesús Camacho Rodríguez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63343/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2017, 8:06 p.m.) > > > Review request for hive, Ashutosh Chauhan and Vineet Garg. > > > Bugs: HIVE-17766 > https://issues.apache.org/jira/browse/HIVE-17766 > > > Repository: hive-git > > > Description > ------- > > HIVE-17766 > > > Diffs > ----- > > itests/src/test/resources/testconfiguration.properties > a4648beae1ec18c88bab35ae3e7f9d9f202e40e5 > ql/src/java/org/apache/hadoop/hive/ql/exec/CommonJoinOperator.java > 3573d07e9a3bab4dabdc0b73292bd02ad05cbc89 > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java > 46493ac006d00f2e6c0054da2438c0b01c1e3fd3 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > 5ccb69ac4824a2002b2bddb1ad206cb7acd3ace3 > ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java > 4ddf1d5b5e87a43f7f606ce3c09ac295f5319333 > ql/src/test/queries/clientpositive/semijoin6.q PRE-CREATION > ql/src/test/results/clientpositive/llap/semijoin6.q.out PRE-CREATION > > > Diff: https://reviews.apache.org/r/63343/diff/5/ > > > Testing > ------- > > > Thanks, > > Jesús Camacho Rodríguez > >
